Re: [evolution-patches] [calendar] fix for #60736

[Not Zed <notzed ximian com>]

On Fri, 2004-07-23 at 08:18 +0200, Rodrigo Moya wrote:
> On Fri, 2004-07-23 at 12:00 +0800, Not Zed wrote:
> > 
> > Will this actually make any difference?
> > 
> > The backtrace doesn't show the idle handler being recursively called,
> > which I presume can't happen anyway.  The idle handler will implictly
> > be removed automagically when it returns FALSE.  So explictly removing
> > it is redundant.
> > 
> there are calls in those idle handlers that could trigger some calls to
> the idle handler again.
I thought glib wont call idle handlers recursively?  If it did the whole idle handler thing wouldn't work very well and you'd be getting inifinite recursion all over the place.

> > That code seems over-complicated for what it does and prone to
> > issues :-/
> > 
> yes, the problem is that both the auth and the signal callbacks need to
> be run on the main thread, since they need to get the signals to the
> client. Do you have any suggestion for making it less complicated?

Well the mailer uses a (global) queue of password requests.  If an existing password request is open it just queues it and goes to sleep, otherwise it queues it and also fires off a message to the main loop to process it (equivalent of adding an idle callback).  After the password is requested it returns the result and checks the queue, if its non-empty it runs another one.  It also has a bit of messy stuff to handle being called from the mainloop without causing deadlocks.

This also lets the password request run asynchronously and outside of any locks.  It doesn't need the lock around the password request since password requests are all serialised (globally) already.  This also stops you getting multiple requests showing up at once and making sure they run in the right order.  And especially stops re-asking for the same value that you've already got (if that issue still exists??).

I would probably:

• Use a global queue and lock for password request handling, but don't lock while you're actually requesting the password.  Well thats if you want the serialisation to happen too.
• Don't pass an allocated structure around which is freed in basically a totally unrelated call.  Probably just use the stack, the password request doesn't need much data.
• I would also add an 'async open' flag to e-cal-private so you don't mess around changing the 'auth_func' pointers for this special case.  Not having to restore those pointers makes things simpler.
• I would also do the idl all synchronous, but thats unrelated/out of scope :)

--

Michael Zucchi <notzed ximian com> "born to die, live to work, it's all downhill from here" Novell's Evolution and Free Software Developer