Re: [PATCH] Timeout support - Please review

[Jules Colding <colding omesc com>]

Hi Michael,

On Tue, 2007-06-12 at 10:02 +0100, Michael Meeks wrote:
> On Mon, 2006-12-04 at 14:22 +0100, Jules Colding wrote:
> > Any and all occurrences of g_cond_wait() has the potential to block
> > forever if the remote server is physically disconnected or powered of.
> 
> 	Urgh - but also, some methods take longer than 30 seconds to execute,
> and will not respond in that time.

Yes, you are right.


> 	So: can we do this for IPv4/6 connections only ? local unix 
> 	    socket connections have strong lifecycle guarentees and this
> 	    covers the desktop use-case nicely.

Yes.


> > +	* src/linc.c (link_wait): Do not wait forever for the link
> > +	condition to get signaled.
> .. 
> > +				if (!g_cond_timed_wait (tdata->incoming, tdata->lock, ((0 < giop_initial_timeout_limit) ? &tval : NULL))) {
> > +					*timeout = TRUE;
> > +					break;
> 
> 	So I'm convinced this code is in the wrong place - worse, the link_wait
> function now -releases- a mutex it didn't take itself: which looks
> horribly un-maintainable: who took that lock ? and worse doing:
> 
> 	if (foo_is_locked())
> 		unlock_foo();
> 
> 	is just grotesquely racey.

Yes, I know. Despite my previous bragging about "flawless quality" I was
actually a little worried about this too...


> 	So - IMHO, the -right- way to implement this is rather simpler:
> 
> 	* in the IO thread, we need to add a simple 'timeout' source to
> 	  the glib mainloop, that will timeout after (however long) and
> 	  in this case just push a constructed CORBA_COMM exception into
> 	  a synthesised reply that we shove into the queue as normal:
> 	  thus signalling the waiting caller.
> 
> 	Surely that is simpler, easier, touches just 1 place, and doesn't
> introduce odd behavior ? :-)

Absolutely. I'll reverse my previous patch and look into this approach
instead. 

Stay tuned,
  jules