RE: ... and the pleasures of locking problems

[Philip Van Hoof <spam pvanhoof be>]

On Wed, 2007-07-25 at 09:50 +0300, Dirk-Jan Binnema nokia com wrote:

> What does this mean on a practical level? Ie.,
> 1) while getting headers for big folder A, I select folder B

If big folder A was already downloaded then big folder A will be
synchronised, else it will be downloaded. A progress bar will be shown.

Once finished, folder B will start loading, a progress bar will be
shown.

The loading of A will show headers as they are retrieved and/or as they
are being synchronised. Same for B. 

> 2) while getting headers folder big folder A, I press 'Cancel'.

After a reconnect, folder B will start loading in that case, as the
cancel outside of the queue is still invoked and effective at this
moment.

Actually it's pretty easy to understand the reason for the current
defect cancels after this change: the cancels are invoked by the
launcher code of the operation. This launcher code is now serialised in
a queue. Therefore wont a registration of a new operation cancel-out the
current operations: The queue will only select it as soon as the current
one is finished. The cancel code will still happen ... but by the time
it runs, there's no more operation to actually cancel.

Any external cancel is still effective and will cancel the current
operation on this queue. Plans to add an API to clear-up this internal
queue are being made. Probably at the level of TnyCamelStoreAccount.

For example a tny_camel_store_account_clear_working_queue (acc).
Combined with a tny_account_cancel would that cancel the current and
clear the entire queue (making an effective "cancel all" feature for all
operations what have so far been requested and therefore registered on
the queue -- NOT for future operations, those are for the application
developer to handle or cancel out himself --).

> I noticed this is 'preparing for a Release Candidate' -
> so no more semantic/architectural changes are expected before 
> that? 

Those preparations where a different changeset. They included
synchronising the enums and flags with the GType registrations for them.

Note:

As the ChangeLog item says: it's a preparation for a release candidate,
not an actual commitment for it. A release of a release candidate is a
non-formal promise not to change API nor ABI anymore. This change didn't
change any API nor ABI, it only changes behaviour. The changed behaviour
is completely within the contract definition of the API too.


> > -----Original Message-----
> > From: tinymail-devel-list-bounces gnome org 
> > [mailto:tinymail-devel-list-bounces gnome org] On Behalf Of 
> > ext Philip Van Hoof
> > Sent: Wednesday, July 25, 2007 02:43
> > To: tinymail-devel-list
> > Subject: ... and the pleasures of locking problems
> > 
> > Tinymail's _async functions where a design error right from the start.
> > Therefore I drastically refactored them. I once also started 
> > libtinymail-queues and the TnyQueue + libtinymail-asyncworker 
> > work to emphasise that the _async functions are indeed a 
> > problem. I figured that because a lot people are nonetheless 
> > going to use them, the big f.
> > design error in Tinymail needs a fix too.
> > 
> > so ...
> > 
> > I introduced an internal private type called TnyCamelQueue in 
> > libtinymail-camel. Right now, in stead of simply launching 
> > threads in the _async_default functions, I call 
> > _tny_camel_queue_launch.
> > 
> > The launch method of TnyCamelQueue is indeed the method to 
> > add an item to the queue.
> > 
> > I haven't defined any contract for this method as it's a 
> > private function. The contract for it can be considered to be:
> > 
> > "Will launch the function and pass data to it in future. Will 
> > immediately return and wont block until the function is 
> > handled. The function can happen in a thread therefore 
> > enforcing you to use locking correctly in your 
> > implementations. Items in the queue will happen serialised. 
> > This means that no items in the queue will be processed in 
> > parallel." ... or something like that.
> > 
> > It does not specify when exactly in future, only that it will 
> > happen in future. There's no priority setting, it's a simple FIFO.
> > 
> > All the callbacks of the existing _async functions are 
> > specified to be called in the mainloop. What makes this 
> > indeed happen is (still) implemented by the execute_callback 
> > function (which got recently refactored by Sergio, in case 
> > you wonder about it) (differently put:
> > nothing got changed).
> > 
> > This might have broken some of the cancels and definitely has 
> > broken support for parallel getting messages on IMAP. The 
> > last one is easily solvable and fixable. The first one, in 
> > case true, is less easy to fix.
> > 
> > I noticed that the first one (broken cancels) doesn't always 
> > occur. I noticed that the second one (broken parallel getting 
> > of messages over
> > IMAP) always does occur. I noticed that on POP parallel 
> > getting of messages is still functional.
> > 
> > Since now _async operations happen serialised, there's a more 
> > clear and clean view over the locking order. The amount of 
> > threads that can be active is also drastically lowered to ... 
> > one for this queue.
> > 
> > I know I'm going to be stormed by panicking project managers 
> > and people with lots of questions. This is better, more 
> > stable and worth temporarily breaking the two functions 
> > (cancels and parallel getting messages).
> > 
> > Although it wont fix all locking problems, it will or should 
> > fix a lot of them or at least finding them and fixing it much 
> > more easy.
> > 
> > More work on also the account initialisation and connecting 
> > will happen soon. This work attempts to get the alert_func, 
> > get_pass_func and forget_pass_func to be ran in the GMainLoop 
> > of your application. This could, at least in theory, also fix 
> > a lot of ui hang difficulties.
> > 
> > 
> > Last but not least: if people disagree that this is the right 
> > way to fix the problems, they can ask me to create a branch 
> > somewhere that doesn't redo the _async function's infrastructure.
> > 
> > We can also agree to disagree by having multiple 
> > implementations of TnyCamelQueue. The first-original 
> > implementation simply calls the g_thread_create API exactly 
> > like how the original code was, just wrapped by the 
> > _tny_camel_queue_launch (makes absolutely no difference).
> > 
> > Note, though, that this affects the behaviour of Tinymail 
> > significantly and that people who disagree will have to 
> > maintain the 'reverse of the refactor' themselves.
> > 
> > 
> > 
> > --
> > Philip Van Hoof, software developer
> > home: me at pvanhoof dot be
> > gnome: pvanhoof at gnome dot org
> > http://www.pvanhoof.be/blog
> > 
> > 
> > 
> > 
> > _______________________________________________
> > tinymail-devel-list mailing list
> > tinymail-devel-list gnome org
> > http://mail.gnome.org/mailman/listinfo/tinymail-devel-list
> > 
> _______________________________________________
> tinymail-devel-list mailing list
> tinymail-devel-list gnome org
> http://mail.gnome.org/mailman/listinfo/tinymail-devel-list
-- 
Philip Van Hoof, software developer
home: me at pvanhoof dot be 
gnome: pvanhoof at gnome dot org 
http://www.pvanhoof.be/blog