Re: [PATCH] Timeout support - Please review
- From: Jules Colding <colding omesc com>
- To: "Gustavo J. A. M. Carneiro" <gjc inescporto pt>
- Cc: ORBit2 <orbit-list gnome org>
- Subject: Re: [PATCH] Timeout support - Please review
- Date: Tue, 05 Dec 2006 13:06:27 +0100
On Tue, 2006-12-05 at 10:47 +0000, Gustavo J. A. M. Carneiro wrote:
> On Ter, 2006-12-05 at 09:50 +0100, Jules Colding wrote:
> > On Mon, 2006-12-04 at 14:22 +0100, Jules Colding wrote:
> > > Please review and test this patch. I'm not entirely convinced that I
> > > haven't introduced a bad bug or two but at least it works(*) here.
> >
> > No responses yet, so it seems that my patch is of the usual flawless
> > quality ;-)
>
> Heh :)
>
> In fact, the patch sounds really nice, a sign of ORBit2 maturing for
> TCP/IP connections (which is traditionally seldom tested).
>
> My only comment is that the changelog entry could use a bit more
> detail. You only mention changes in one function, but the patch changes
> 4 functions and even adds a new commandline option which deserves a
> mention in the changelog too.
Here is the new and better documented patch.
Best regards,
jules
Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/ORBit2/ChangeLog,v
retrieving revision 1.778
diff -u -p -r1.778 ChangeLog
--- ChangeLog 22 Nov 2006 20:34:39 -0000 1.778
+++ ChangeLog 5 Dec 2006 12:04:04 -0000
@@ -1,3 +1,95 @@
+2006-12-05 Jules Colding <colding omesc com>
+
+ * ORBit2: The previous two ChangeLog entries are hiding a lot of
+ details which I'll hereby try to expand upon.
+
+ The rationale for these changes is that any and all occurrences of
+ g_cond_wait() in ORBit2 has the potential to block forever if it
+ waits for a connection attempt to complete if said connection
+ attempt is made towards a remote server which happens to be
+ physically disconnected or powered off.
+
+ This blocking behavior can be demonstrated by invoking an operation
+ on a remote object that is physically inaccessible such as when
+ the remote server is powered down or physically disconnected.
+
+ Pseudo code:
+
+ {
+ CORBA_ORB orb = get_orb();
+ char *objref = get_corbaloc_str();
+ CORBA_Object obj = CORBA_OBJECT_NIL;
+ CORBA_Environment ev[1];
+
+ CORBA_exception_init(ev);
+ obj = CORBA_ORB_string_to_object(orb, objref, ev);
+
+ FOO_INTERFACE_method(obj, <arguments>, ev);
+ }
+
+ The last statement above will block forever in g_cond_wait() while
+ waiting for the recieve buffer to be ready if the remote server is,
+ say, powered down. An exception is on the other hand immediately
+ thrown if the server object is merely gone.
+
+ The changes to the code therefore boils down to the implementation
+ of the ex_CORBA_TIMEOUT system exception being thrown if
+ g_cond_timed_wait() times out. Most occurrences of g_cond_wait()
+ has been replaced by g_cond_timed_wait().
+
+ The only remaining instance of g_cond_wait() is found in
+ link_exec_command(). I haven't observed any problems with
+ link_exec_command() so I've not touched that code.
+
+ The affected code is:
+
+ 1) A new ORB command line option: GIOPTimeoutLimit - timeout limit
+ in microseconds. Defaults to 30 seconds.
+
+ 2) giop_recv_buffer_get(). This one is obvious. We do not want to
+ wait indefinitely for data to be received from a downed host. The
+ waiting interval is configurable with the new GIOPTimeoutLimit ORB
+ option.
+
+ 3) Any code invoking link_wait():
+
+ a) link_connection_wait_connected_T()
+ b) link_connection_try_reconnect()
+ c) giop_connection_try_reconnect(), calls link_connection_try_reconnect()
+ d) ORBit_try_connection_T(), calls giop_connection_try_reconnect()
+ e) ORBit_object_get_connection(), calls ORBit_try_connection_T()
+
+ 4) The waiting time in link_wait() is set at compile time to
+ LINK_WAIT_TIMEOUT_USEC which is presently 10 seconds.
+
+ 5) link_wait() has been modified to return a gboolean. FALSE if the
+ timeout expired, TRUE otherwise.
+
+ 6) link_connection_wait_connected_T() and link_connection_try_reconnect()
+ will both disconnect the connection if link_wait() experienced a timeout.
+
+2006-12-04 Jules Colding <colding omesc com>
+
+ * configure.in: Added autoconf 2.60+ required datarootdir variable
+
+2006-12-03 Jules Colding <colding omesc com>
+
+ * include/orbit/GIOP/giop.h: Declare giop_recv_set_timeout().
+
+ * src/orb/orb-core/corba-orb.c (CORBA_ORB_init): Set GIOP timeout
+ limit from the ORB option.
+ Added new ORB option - GIOPTimeoutLimit.
+
+ * src/orb/orb-core/orbit-small.c (ORBit_small_invoke_stub): Throw a
+ TIMEOUT exception if a reply hasn't been recieved within the GIOP
+ timeout limit.
+
+ * src/orb/GIOP/giop-recv-buffer.c (giop_recv_buffer_get): Replaced
+ a g_cond_wait() with a g_cond_timed_wait(). The original g_cond_wait()
+ could potentially block forever if a remote server was physically
+ offline.
+ (giop_recv_set_timeout): Function to adjust the GIOP timeout limit.
+
2006-11-22 Kjartan Maraas <kmaraas gnome org>
* ORBit-2.0.pc.in: Move MINGW_CFLAGS and LIBM to Libs.private
Index: configure.in
===================================================================
RCS file: /cvs/gnome/ORBit2/configure.in,v
retrieving revision 1.175
diff -u -p -r1.175 configure.in
--- configure.in 22 Nov 2006 20:34:41 -0000 1.175
+++ configure.in 5 Dec 2006 12:04:04 -0000
@@ -37,6 +37,9 @@ AM_CONFIG_HEADER(config.h)
dnl Initialize automake stuff
AM_INIT_AUTOMAKE(ORBit2, $ORBIT_VERSION, no-define)
+dnl Required by autoconf 2.60
+AC_SUBST(datarootdir)
+
AC_CANONICAL_HOST
AC_MSG_CHECKING([for Win32])
case "$host" in
Index: include/orbit/GIOP/giop-recv-buffer.h
===================================================================
RCS file: /cvs/gnome/ORBit2/include/orbit/GIOP/giop-recv-buffer.h,v
retrieving revision 1.33
diff -u -p -r1.33 giop-recv-buffer.h
--- include/orbit/GIOP/giop-recv-buffer.h 14 Jan 2004 11:04:01 -0000 1.33
+++ include/orbit/GIOP/giop-recv-buffer.h 5 Dec 2006 12:04:04 -0000
@@ -58,7 +58,8 @@ void giop_recv_list_setup_que
void giop_recv_list_setup_queue_entry_async (GIOPMessageQueueEntry *ent,
GIOPAsyncCallback cb);
-GIOPRecvBuffer *giop_recv_buffer_get (GIOPMessageQueueEntry *ent);
+GIOPRecvBuffer *giop_recv_buffer_get (GIOPMessageQueueEntry *ent,
+ gboolean *timeout);
void giop_recv_buffer_unuse (GIOPRecvBuffer *buf);
#define giop_recv_buffer_reply_status(buf) ( \
Index: include/orbit/GIOP/giop-types.h
===================================================================
RCS file: /cvs/gnome/ORBit2/include/orbit/GIOP/giop-types.h,v
retrieving revision 1.21
diff -u -p -r1.21 giop-types.h
--- include/orbit/GIOP/giop-types.h 27 Oct 2003 16:14:12 -0000 1.21
+++ include/orbit/GIOP/giop-types.h 5 Dec 2006 12:04:04 -0000
@@ -35,7 +35,9 @@ struct _GIOPThread {
gpointer dummy);
};
-#define GIOP_INITIAL_MSG_SIZE_LIMIT 256*1024
+#define GIOP_INITIAL_TIMEOUT_LIMIT (30000000) /* 30 seconds */
+
+#define GIOP_INITIAL_MSG_SIZE_LIMIT (256*1024) /* in bytes */
typedef enum {
GIOP_CONNECTION_SSL
Index: include/orbit/GIOP/giop.h
===================================================================
RCS file: /cvs/gnome/ORBit2/include/orbit/GIOP/giop.h,v
retrieving revision 1.24
diff -u -p -r1.24 giop.h
--- include/orbit/GIOP/giop.h 5 Apr 2006 07:16:10 -0000 1.24
+++ include/orbit/GIOP/giop.h 5 Dec 2006 12:04:04 -0000
@@ -23,6 +23,7 @@ gboolean giop_thread_safe (void
gboolean giop_thread_io (void);
GIOPThread *giop_thread_self (void);
void giop_invoke_async (GIOPMessageQueueEntry *ent);
+void giop_recv_set_timeout (const glong timeout);
void giop_recv_set_limit (glong limit);
glong giop_recv_get_limit (void);
void giop_incoming_signal_T (GIOPThread *tdata, GIOPMsgType t);
@@ -45,6 +46,7 @@ void giop_thread_new_check
void giop_thread_queue_process (GIOPThread *tdata);
gboolean giop_thread_queue_empty_T (GIOPThread *tdata);
void giop_thread_queue_tail_wakeup(GIOPThread *tdata);
+
#endif /* ORBIT2_INTERNAL_API */
Index: linc2/ChangeLog
===================================================================
RCS file: /cvs/gnome/ORBit2/linc2/ChangeLog,v
retrieving revision 1.262
diff -u -p -r1.262 ChangeLog
--- linc2/ChangeLog 22 Nov 2006 19:13:41 -0000 1.262
+++ linc2/ChangeLog 5 Dec 2006 12:04:05 -0000
@@ -1,3 +1,8 @@
+2006-12-04 Jules Colding <colding omesc com>
+
+ * src/linc.c (link_wait): Do not wait forever for the link
+ condition to get signaled.
+
2006-11-22 Kjartan Maraas <kmaraas gnome org>
* src/Makefile.am: Remove SSL_LIBS and SSL_CFLAGS.
Index: linc2/include/linc/linc.h
===================================================================
RCS file: /cvs/gnome/ORBit2/linc2/include/linc/linc.h,v
retrieving revision 1.17
diff -u -p -r1.17 linc.h
--- linc2/include/linc/linc.h 28 Jan 2005 12:34:57 -0000 1.17
+++ linc2/include/linc/linc.h 5 Dec 2006 12:04:05 -0000
@@ -33,7 +33,7 @@ GMainLoop *link_main_get_loop (void);
guint link_main_idle_add (GSourceFunc function,
gpointer data);
-void link_wait (void);
+gboolean link_wait (void);
void link_signal (void);
gboolean link_thread_io (void);
Index: linc2/src/linc-connection.c
===================================================================
RCS file: /cvs/gnome/ORBit2/linc2/src/linc-connection.c,v
retrieving revision 1.117
diff -u -p -r1.117 linc-connection.c
--- linc2/src/linc-connection.c 9 Sep 2006 07:54:37 -0000 1.117
+++ linc2/src/linc-connection.c 5 Dec 2006 12:04:06 -0000
@@ -635,8 +635,10 @@ link_connection_do_initiate (LinkConnect
static LinkConnectionStatus
link_connection_wait_connected_T (LinkConnection *cnx)
{
- while (cnx && cnx->status == LINK_CONNECTING)
- link_wait ();
+ while (cnx && cnx->status == LINK_CONNECTING) {
+ if (!link_wait ())
+ link_connection_disconnect (cnx);
+ }
return cnx ? cnx->status : LINK_DISCONNECTED;
}
@@ -659,8 +661,12 @@ link_connection_try_reconnect (LinkConne
cnx->inhibit_reconnect = FALSE;
dispatch_callbacks_drop_lock (cnx);
g_main_context_release (NULL);
- } else
- link_wait ();
+ } else {
+ if (!link_wait ()) {
+ link_connection_disconnect (cnx);
+ break;
+ }
+ }
}
if (cnx->status != LINK_DISCONNECTED)
Index: linc2/src/linc.c
===================================================================
RCS file: /cvs/gnome/ORBit2/linc2/src/linc.c,v
retrieving revision 1.63
diff -u -p -r1.63 linc.c
--- linc2/src/linc.c 2 Jun 2006 08:14:22 -0000 1.63
+++ linc2/src/linc.c 5 Dec 2006 12:04:06 -0000
@@ -52,6 +52,9 @@ SSL_METHOD *link_ssl_method;
SSL_CTX *link_ssl_ctx;
#endif
+/* max time to wait for the link condition to get signaled - 10 seconds */
+#define LINK_WAIT_TIMEOUT_USEC (10000000)
+
static void link_dispatch_command (gpointer data, gboolean immediate);
gboolean
@@ -534,17 +537,28 @@ link_signal (void)
}
}
-void
+gboolean
link_wait (void)
{
+ GTimeVal gtime;
+
if (!(link_is_thread_safe && link_is_io_in_thread)) {
link_unlock ();
link_main_iteration (TRUE);
link_lock ();
} else {
g_assert (link_main_cond != NULL);
- g_cond_wait (link_main_cond, link_main_lock);
+
+ g_get_current_time (>ime);
+ g_time_val_add (>ime, LINK_WAIT_TIMEOUT_USEC);
+ if (!g_cond_timed_wait (link_main_cond, link_main_lock, >ime)) {
+ if (link_is_locked ())
+ link_unlock ();
+ return FALSE;
+ }
}
+
+ return TRUE;
}
Index: src/orb/GIOP/giop-recv-buffer.c
===================================================================
RCS file: /cvs/gnome/ORBit2/src/orb/GIOP/giop-recv-buffer.c,v
retrieving revision 1.81
diff -u -p -r1.81 giop-recv-buffer.c
--- src/orb/GIOP/giop-recv-buffer.c 5 Apr 2006 07:17:17 -0000 1.81
+++ src/orb/GIOP/giop-recv-buffer.c 5 Dec 2006 12:04:07 -0000
@@ -690,10 +690,21 @@ check_got (GIOPMessageQueueEntry *ent)
(ent->cnx->parent.status == LINK_DISCONNECTED));
}
+static glong giop_initial_timeout_limit = GIOP_INITIAL_TIMEOUT_LIMIT;
+
+void
+giop_recv_set_timeout (const glong timeout)
+{
+ if (0 < timeout) /* We really do not want (timeout <= 0) as that would potentially block forever */
+ giop_initial_timeout_limit = timeout;
+}
+
GIOPRecvBuffer *
-giop_recv_buffer_get (GIOPMessageQueueEntry *ent)
+giop_recv_buffer_get (GIOPMessageQueueEntry *ent,
+ gboolean *timeout)
{
GIOPThread *tdata = giop_thread_self ();
+ GTimeVal tval;
thread_switch:
if (giop_thread_io ()) {
@@ -704,8 +715,17 @@ giop_recv_buffer_get (GIOPMessageQueueEn
ent_unlock (ent);
giop_thread_queue_process (tdata);
ent_lock (ent);
- } else
- g_cond_wait (tdata->incoming, tdata->lock);
+ } else {
+ if (0 < giop_initial_timeout_limit) {
+ g_get_current_time (&tval);
+ g_time_val_add (&tval, giop_initial_timeout_limit);
+ }
+ if (!g_cond_timed_wait (tdata->incoming, tdata->lock, ((0 < giop_initial_timeout_limit) ? &tval : NULL))) {
+ *timeout = TRUE;
+ break;
+ } else
+ *timeout = FALSE;
+ }
}
ent_unlock (ent);
@@ -1352,3 +1372,4 @@ giop_recv_buffer_use_buf (GIOPConnection
return buf;
}
+
Index: src/orb/orb-core/corba-orb.c
===================================================================
RCS file: /cvs/gnome/ORBit2/src/orb/orb-core/corba-orb.c,v
retrieving revision 1.123
diff -u -p -r1.123 corba-orb.c
--- src/orb/orb-core/corba-orb.c 16 Aug 2006 09:28:30 -0000 1.123
+++ src/orb/orb-core/corba-orb.c 5 Dec 2006 12:04:07 -0000
@@ -61,7 +61,7 @@ static char *orbit_debug_options
static char *orbit_naming_ref = NULL;
static GSList *orbit_initref_list = NULL;
static gboolean orbit_use_corbaloc = FALSE;
-
+static gint orbit_timeout_limit = -1;
void
ORBit_ORB_start_servers (CORBA_ORB orb)
{
@@ -417,8 +417,8 @@ CORBA_ORB_init (int *argc, char **argv,
}
#endif /* G_ENABLE_DEBUG */
-
giop_recv_set_limit (orbit_initial_recv_limit);
+ giop_recv_set_timeout (orbit_timeout_limit);
giop_init (thread_safe,
orbit_use_ipv4 || orbit_use_ipv6 ||
orbit_use_irda || orbit_use_ssl);
@@ -1467,6 +1467,7 @@ const ORBit_option orbit_supported_optio
{ "ORBDebugFlags", ORBIT_OPTION_STRING, &orbit_debug_options },
{ "ORBInitRef", ORBIT_OPTION_KEY_VALUE, &orbit_initref_list},
{ "ORBCorbaloc", ORBIT_OPTION_BOOLEAN, &orbit_use_corbaloc},
+ { "GIOPTimeoutLimit", ORBIT_OPTION_INT, &orbit_timeout_limit },
{ NULL, 0, NULL },
};
Index: src/orb/orb-core/orbit-small.c
===================================================================
RCS file: /cvs/gnome/ORBit2/src/orb/orb-core/orbit-small.c,v
retrieving revision 1.97
diff -u -p -r1.97 orbit-small.c
--- src/orb/orb-core/orbit-small.c 18 May 2006 14:20:49 -0000 1.97
+++ src/orb/orb-core/orbit-small.c 5 Dec 2006 12:04:08 -0000
@@ -591,6 +591,7 @@ ORBit_small_invoke_stub (CORBA_Object
GIOPRecvBuffer *recv_buffer = NULL;
CORBA_Object xt_proxy = CORBA_OBJECT_NIL;
ORBitPolicy *invoke_policy = CORBA_OBJECT_NIL;
+ gboolean timeout = FALSE;
if (!obj) {
dprintf (MESSAGES, "Cannot invoke method on null object\n");
@@ -654,7 +655,9 @@ ORBit_small_invoke_stub (CORBA_Object
goto clean_out;
}
- recv_buffer = giop_recv_buffer_get (&mqe);
+ recv_buffer = giop_recv_buffer_get (&mqe, &timeout);
+ if (timeout)
+ goto timeout_exception;
switch (orbit_small_demarshal (obj, &cnx, recv_buffer, ev,
ret, m_data, args))
@@ -698,6 +701,12 @@ ORBit_small_invoke_stub (CORBA_Object
tprintf ("[System exception comm failure] )");
CORBA_exception_set_system (ev, ex_CORBA_COMM_FAILURE,
completion_status);
+ goto clean_out;
+
+ timeout_exception:
+ tprintf ("[System exception timeout] )");
+ CORBA_exception_set_system (ev, ex_CORBA_TIMEOUT,
+ CORBA_COMPLETED_NO);
goto clean_out;
}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
