Re: [PATCH] Timeout support - Please review

[Michael Meeks <michael ximian com>]

Hi Jules,

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.

	Also - when you are debugging, it is extremely normal to want to pause
everything in the debugger to examine some interaction - and you don't
expect everything to fail in unpredictable (or even silent ways) when
you continue.

	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.

> +	* 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.

	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 ? :-)

	Thanks,

		Michael.

PS. if you really want patch review, can you CC me explicitly.
-- 
 michael meeks novell com  <><, Pseudo Engineer, itinerant idiot