New GIOP timeout patch - please review

[timeout.h]
/*

 * CORBA timeout test values

 *

 * This program is free software; you can redistribute it and/or modify it

 * under the terms of the GNU General Public License as published by the

 * Free Software Foundation; either version 2, or (at your option) any

 * later version.

 *

 * This program is distributed in the hope that it will be useful,

 * but WITHOUT ANY WARRANTY; without even the implied warranty of

 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

 * GNU General Public License for more details.

 *

 * You should have received a copy of the GNU General Public License

 * along with this program; if not, write to the Free Software Foundation,

 * Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.

 *

 * Author: Jules Colding <colding@.com>

 */

#ifndef timeout_H

#define timeout_H (1)

#define TIMEOUT (2)

#define SLEEP   (TIMEOUT+1)

#define TIMEOUT_STR "test_timeout"

#endif

[for-jules.diff]

	Re: New GIOP timeout patch - please review by michael meeks :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message Hi Jules, On Thu, 2007-06-21 at 14:49 +0200, Jules Colding wrote: > As always... Just noticed two small and stupid bugs. Corrected patch > below. Sigh - so 1 new thread per IPv4 connection ? that will cripple Sun who use IPv4/CORBA to do their Java a11y bridging stuff. We should only do that as a stop-gap, and only if it's turned off by default for IPv4 users I think [ is that so ? ]. -really- it would be way better to instrument your glib and find out why the timeout thing is not getting called. -One- possibility may be that you're not setting it to be a 'recursive' source (although perhaps unlikely given the lack of re-enterancy there). HTH, Michael. -- michael.meeks@... <><, Pseudo Engineer, itinerant idiot _______________________________________________ orbit-list mailing list orbit-list@... http://mail.gnome.org/mailman/listinfo/orbit-list
	New GIOP timeout patch - please review by colding :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message Hi all, I've reverted my previous timeout approach following well deserved criticism from Michael. This new approach seems to do the right thing and I've tested it with a modified echo client/server. Explanation and patch below. Please comment. Thanks, jules The patch does the following: 0) The old timeout patch is reverted. 1) The timeout value is initialized in ORBit_ORB_start_servers(). Explicitly setting the value to 0 (zero) will disable timeout altogether. The default value is 60 seconds. 2) The timeout value is a guint. I've added support for guint ORB options. 3) Timeout helper members has been added to the LinkConnection structure. This includes a timeout GMutex, a new link enum status member and the timeout value in milliseconds. 4) These timeout members are initialized from there NIL state to something that will result in an effective timeout on the individual connection. This happens iff these statements hold true: a) The connection is IPv4 or IPv6 b) The timeout member value is non-zero This initialization happens in link_connection_from_fd_T(). 5) A timeout thread is effectively launched in giop_send_buffer_write() but only if the connection is not a oneway. 6) The timeout thread function will set the link state to LINK_TIMEOUT and signal the GCond that giop_recv_buffer_get() waits on. 7) ORBit_small_invoke_stub() will push a timeout exception down the line upon learning from giop_recv_buffer_get() that the connection is timed out. Index: src/orb/orb-core/corba-orb.c =================================================================== --- src/orb/orb-core/corba-orb.c (revision 2003) +++ src/orb/orb-core/corba-orb.c (working copy) @@ -61,7 +61,7 @@ static char orbit_naming_ref = NULL; static GSList orbit_initref_list = NULL; static gboolean orbit_use_corbaloc = FALSE; -static gint orbit_timeout_limit = -1; +static guint orbit_timeout_msec = 60000; /* 60 seconds - 0 will disable timeouts altogether / void ORBit_ORB_start_servers (CORBA_ORB orb) { @@ -418,7 +418,7 @@ #endif / G_ENABLE_DEBUG / giop_recv_set_limit (orbit_initial_recv_limit); - giop_recv_set_timeout (orbit_timeout_limit); + giop_set_timeout (orbit_timeout_msec); giop_init (thread_safe, orbit_use_ipv4 \|\| orbit_use_ipv6 \|\| orbit_use_irda \|\| orbit_use_ssl); @@ -1468,7 +1468,7 @@ { "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 }, + { "GIOPTimeoutMSEC", ORBIT_OPTION_ULONG, &orbit_timeout_msec }, { NULL, 0, NULL } }; Index: src/orb/orb-core/corba-object.c =================================================================== --- src/orb/orb-core/corba-object.c (revision 2003) +++ src/orb/orb-core/corba-object.c (working copy) @@ -270,6 +270,7 @@ retval = TRUE; break; case LINK_DISCONNECTED: + case LINK_TIMEOUT: / Have a go at reviving it / dprintf (MESSAGES, "re-connecting dropped cnx %p: ", cnx); if (giop_connection_try_reconnect (GIOP_CONNECTION (cnx)) == LINK_CONNECTED) Index: src/orb/util/orbit-options.c =================================================================== --- src/orb/util/orbit-options.c (revision 2003) +++ src/orb/util/orbit-options.c (working copy) @@ -53,6 +53,9 @@ case ORBIT_OPTION_INT: (gint )option->arg = atoi (val); break; + case ORBIT_OPTION_ULONG: + (guint )option->arg = strtoul(val, (char )NULL, 10); + break; case ORBIT_OPTION_STRING: { gchar str_arg = (char ) option->arg; Index: src/orb/util/orbit-options.h =================================================================== --- src/orb/util/orbit-options.h (revision 2003) +++ src/orb/util/orbit-options.h (working copy) @@ -10,7 +10,8 @@ ORBIT_OPTION_STRING, ORBIT_OPTION_INT, ORBIT_OPTION_BOOLEAN, - ORBIT_OPTION_KEY_VALUE / returns GSList of ORBit_option_key_value / + ORBIT_OPTION_KEY_VALUE, / returns GSList of ORBit_option_key_value / + ORBIT_OPTION_ULONG, } ORBit_option_type; typedef struct { Index: src/orb/GIOP/giop-recv-buffer.c =================================================================== --- src/orb/GIOP/giop-recv-buffer.c (revision 2003) +++ src/orb/GIOP/giop-recv-buffer.c (working copy) @@ -686,26 +686,20 @@ static inline gboolean check_got (GIOPMessageQueueEntry ent) { - return (ent->buffer \|\| !ent->cnx \|\| - (ent->cnx->parent.status == LINK_DISCONNECTED)); + return (ent->buffer \|\| + !ent->cnx \|\| + (ent->cnx->parent.status == LINK_DISCONNECTED) \|\| + (ent->cnx->parent.status == LINK_TIMEOUT)); } -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, gboolean timeout) { GIOPThread tdata = giop_thread_self (); - GTimeVal tval; + timeout = FALSE; + thread_switch: if (giop_thread_io ()) { ent_lock (ent); @@ -715,17 +709,8 @@ ent_unlock (ent); giop_thread_queue_process (tdata); ent_lock (ent); - } 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; - } + } else + g_cond_wait (tdata->incoming, tdata->lock); } ent_unlock (ent); @@ -734,6 +719,7 @@ while (!ent->buffer && ent->cnx && (ent->cnx->parent.status != LINK_DISCONNECTED) && + (ent->cnx->parent.status != LINK_TIMEOUT) && !giop_thread_io()) link_main_iteration (TRUE); @@ -741,6 +727,13 @@ goto thread_switch; } + g_mutex_lock (ent->cnx->parent.timeout_mutex); + if (ent->cnx->parent.timeout_status == LINK_TIMEOUT_UNKNOWN) + ent->cnx->parent.timeout_status = LINK_TIMEOUT_NO; + else + timeout = TRUE; + g_mutex_unlock (ent->cnx->parent.timeout_mutex); + giop_thread_queue_tail_wakeup (tdata); giop_recv_list_destroy_queue_entry (ent); @@ -1355,6 +1348,73 @@ return TRUE; } +struct timeout_thread_data { + guint msec; + GIOPThread tdata; + LinkConnection lcnx; +}; + +static gpointer +giop_timeout(gpointer data) +{ + guint msec = ((struct timeout_thread_data)data)->msec; + LinkConnection lcnx = ((struct timeout_thread_data)data)->lcnx; + GIOPThread tdata = ((struct timeout_thread_data)data)->tdata; + struct timespec tv; + + g_assert (lcnx->timeout_mutex); + + tv.tv_sec = msec / 1000; + tv.tv_nsec = (msec - tv.tv_sec1000) 1000000; + + nanosleep (&tv, NULL); + + g_mutex_lock (lcnx->timeout_mutex); + if (lcnx->timeout_status == LINK_TIMEOUT_UNKNOWN) + lcnx->timeout_status = LINK_TIMEOUT_YES; + else { + g_mutex_unlock (lcnx->timeout_mutex); + goto out; + } + g_mutex_unlock (lcnx->timeout_mutex); + + link_connection_state_changed (lcnx, LINK_TIMEOUT); + + g_mutex_lock (tdata->lock); /* ent_lock / + giop_incoming_signal_T (tdata, GIOP_CLOSECONNECTION); + g_mutex_unlock (tdata->lock); / ent_lock / + +out: + g_object_unref (lcnx); + g_free (data); + + return NULL; +} + +void +giop_timeout_add(GIOPConnection cnx) +{ + struct timeout_thread_data data = NULL; + LinkConnection lcnx = LINK_CONNECTION (cnx); + + if (!lcnx->timeout_msec) + return; + + g_object_ref (lcnx); + + lcnx->timeout_mutex = g_mutex_new (); + + data = g_new0 (struct timeout_thread_data, 1); + data->msec = lcnx->timeout_msec; + data->tdata = giop_thread_self (); + data->lcnx = lcnx; + + g_thread_create (giop_timeout, + (gpointer)data, + FALSE, + NULL); +} + GIOPRecvBuffer * giop_recv_buffer_use_buf (GIOPConnection cnx) { @@ -1372,4 +1432,3 @@ return buf; } - Index: src/orb/GIOP/giop-connection.c =================================================================== --- src/orb/GIOP/giop-connection.c (revision 2003) +++ src/orb/GIOP/giop-connection.c (working copy) @@ -108,6 +108,7 @@ } } + static void giop_connection_class_init (GIOPConnectionClass klass) { @@ -192,3 +193,9 @@ return link_connection_try_reconnect (LINK_CONNECTION (cnx)); } +void +giop_set_timeout (guint msec) +{ + link_set_timeout (msec); +} + Index: src/orb/GIOP/giop-send-buffer.c =================================================================== --- src/orb/GIOP/giop-send-buffer.c (revision 2003) +++ src/orb/GIOP/giop-send-buffer.c (working copy) @@ -45,6 +45,25 @@ static const GIOP_AddressingDisposition giop_1_2_target_type = GIOP_KeyAddr; +static gboolean +giop_send_buffer_is_oneway(const GIOPSendBuffer buf) +{ + g_assert (buf); + + switch (buf->giop_version) { + case GIOP_1_0: + case GIOP_1_1: + return (buf->msg.u.request_1_0.response_expected ? FALSE : TRUE); + case GIOP_1_2: + return (buf->msg.u.request_1_2.response_flags ? FALSE : TRUE); + default: + break; + } + g_assert_not_reached(); + + return TRUE; +} + GIOPSendBuffer giop_send_buffer_use_request (GIOPVersion giop_version, CORBA_unsigned_long request_id, @@ -426,6 +445,7 @@ gboolean blocking) { int retval; + LinkConnection lcnx = LINK_CONNECTION (cnx); static LinkWriteOpts non_block = NULL; if (!non_block) @@ -434,11 +454,15 @@ /* FIXME: if a FRAGMENT, assert the 8 byte tail align, &&\|\| giop_send_buffer_align (buf, 8); / - retval = link_connection_writev ( - (LinkConnection ) cnx, buf->iovecs, - buf->num_used, - blocking ? NULL : non_block); + if (lcnx->timeout_msec && !giop_send_buffer_is_oneway (buf)) { + giop_timeout_add (cnx); + } + retval = link_connection_writev (lcnx, + buf->iovecs, + buf->num_used, + blocking ? NULL : non_block); + if (!blocking && retval == LINK_IO_QUEUED_DATA) retval = 0; Index: linc2/include/linc/linc-connection.h =================================================================== --- linc2/include/linc/linc-connection.h (revision 2003) +++ linc2/include/linc/linc-connection.h (working copy) @@ -39,7 +39,8 @@ #define LINK_IS_CONNECTION(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), LINK_TYPE_CONNECTION)) #define LINK_IS_CONNECTION_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), LINK_TYPE_CONNECTION)) -typedef enum { LINK_CONNECTING, LINK_CONNECTED, LINK_DISCONNECTED } LinkConnectionStatus; +typedef enum { LINK_CONNECTING, LINK_CONNECTED, LINK_DISCONNECTED, LINK_TIMEOUT } LinkConnectionStatus; +typedef enum { LINK_TIMEOUT_UNKNOWN, LINK_TIMEOUT_YES, LINK_TIMEOUT_NO } LinkTimeoutStatus; typedef struct _LinkWriteOpts LinkWriteOpts; typedef struct _LinkConnectionPrivate LinkConnectionPrivate; @@ -61,6 +62,10 @@ LinkConnectionPrivate priv; GSList idle_broken_callbacks; + + GMutex timeout_mutex; + guint timeout_msec; + LinkTimeoutStatus timeout_status;; } LinkConnection; typedef struct { @@ -153,6 +158,9 @@ void link_connections_close (void); +/ set the link timeout in miliseconds / +extern void link_set_timeout (guint msec); + G_END_DECLS #endif / _LINK_CONNECTION_H / Index: linc2/include/linc/linc.h =================================================================== --- linc2/include/linc/linc.h (revision 2003) +++ linc2/include/linc/linc.h (working copy) @@ -33,7 +33,7 @@ guint link_main_idle_add (GSourceFunc function, gpointer data); -gboolean link_wait (void); +void link_wait (void); void link_signal (void); gboolean link_thread_io (void); Index: linc2/ChangeLog =================================================================== --- linc2/ChangeLog (revision 2003) +++ linc2/ChangeLog (working copy) @@ -1,3 +1,24 @@ +2007-06-19 Jules Colding <colding@...> + + src/linc-connection.c (link_connection_init): Initialize + timeout members in the LinkConnection structure. + (link_set_timeout): New function to set the timeout value. + +2007-06-18 Jules Colding <colding@...> + + * src/linc-connection.c (link_connection_from_fd_T): Initiate + timeout members of the link connection iff: + 1) The connection is IPv4 or IPv6 + 2) It is not a oneway + 3) The timeout parameter is non-zero + + * include/linc/linc-connection.h (struct): + 1) timeout mutex + 2) timeout value in milliseconds + 3) timeout status + + Furthermore declare link_set_timeout() + ========================== ORBit2-2.14.8 ======================== 2007-02-27 Kjartan Maraas <kmaraas@...> Index: linc2/src/linc-debug.h =================================================================== --- linc2/src/linc-debug.h (revision 2003) +++ linc2/src/linc-debug.h (working copy) @@ -25,6 +25,7 @@ # define STATE_NAME(s) (((s) == LINK_CONNECTED) ? "Connected" : \ ((s) == LINK_CONNECTING) ? "Connecting" : \ ((s) == LINK_DISCONNECTED) ? "Disconnected" : \ + ((s) == LINK_TIMEOUT) ? "Timeout" : \ "Invalid state") # ifdef CONNECTION_DEBUG_FLAG extern gboolean link_connection_debug_flag; Index: linc2/src/linc-connection.c =================================================================== --- linc2/src/linc-connection.c (revision 2003) +++ linc2/src/linc-connection.c (working copy) @@ -27,6 +27,7 @@ #include <linc/linc-connection.h> static GObjectClass parent_class = NULL; +static guint _link_timeout = 0; enum { BROKEN, @@ -288,6 +289,7 @@ break; case LINK_DISCONNECTED: + case LINK_TIMEOUT: link_source_remove (cnx); link_close_fd (cnx); queue_free (cnx); @@ -440,6 +442,19 @@ g_free (cnx->remote_serv_info); cnx->remote_serv_info = remote_serv_info; + switch (cnx->proto->family) { + case AF_INET: + case AF_INET6: + if (_link_timeout && !cnx->timeout_msec) { / this should'nt happen twice but I'm always paranoid... / + cnx->timeout_mutex = NULL; + cnx->timeout_msec = _link_timeout; + cnx->timeout_status = LINK_TIMEOUT_UNKNOWN; + } + break; + default: + break; + } + d_printf ("Cnx from fd (%d) '%s', '%s', '%s'\n", fd, proto->name, remote_host_info ? remote_host_info : "<Null>", @@ -635,10 +650,8 @@ static LinkConnectionStatus link_connection_wait_connected_T (LinkConnection cnx) { - while (cnx && cnx->status == LINK_CONNECTING) { - if (!link_wait ()) - link_connection_disconnect (cnx); - } + while (cnx && cnx->status == LINK_CONNECTING) + link_wait (); return cnx ? cnx->status : LINK_DISCONNECTED; } @@ -661,12 +674,8 @@ cnx->inhibit_reconnect = FALSE; dispatch_callbacks_drop_lock (cnx); g_main_context_release (NULL); - } else { - if (!link_wait ()) { - link_connection_disconnect (cnx); - break; - } - } + } else + link_wait (); } if (cnx->status != LINK_DISCONNECTED) @@ -1254,6 +1263,9 @@ g_free (cnx->priv); + if (cnx->timeout_mutex) + g_mutex_free (cnx->timeout_mutex); + #ifdef G_ENABLE_DEBUG g_assert (g_list_find(cnx_list, cnx) == NULL); #endif @@ -1269,6 +1281,11 @@ cnx->priv = g_new0 (LinkConnectionPrivate, 1); cnx->priv->fd = -1; cnx->priv->was_disconnected = FALSE; + + cnx->timeout_mutex = NULL; + cnx->timeout_msec = 0; + cnx->timeout_status = LINK_TIMEOUT_UNKNOWN; + #ifdef CONNECTION_DEBUG cnx->priv->total_read_bytes = 0; cnx->priv->total_written_bytes = 0; @@ -1568,3 +1585,10 @@ g_list_free (cnx); } + +void +link_set_timeout (guint msec) +{ + _link_timeout = msec; +} + Index: linc2/src/linc.c =================================================================== --- linc2/src/linc.c (revision 2003) +++ linc2/src/linc.c (working copy) @@ -52,9 +52,6 @@ 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 @@ -537,28 +534,17 @@ } } -gboolean +void 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_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; - } + g_cond_wait (link_main_cond, link_main_lock); } - - return TRUE; } Index: include/orbit/GIOP/giop-connection.h =================================================================== --- include/orbit/GIOP/giop-connection.h (revision 2003) +++ include/orbit/GIOP/giop-connection.h (working copy) @@ -53,6 +53,9 @@ #define giop_connection_ref(cnx) link_connection_ref(cnx) #define giop_connection_unref(cnx) link_connection_unref(cnx) +/ set the link timeout in milliseconds / +extern void giop_set_timeout (guint msec); + #endif / ORBIT2_INTERNAL_API / G_END_DECLS Index: include/orbit/GIOP/giop.h =================================================================== --- include/orbit/GIOP/giop.h (revision 2003) +++ include/orbit/GIOP/giop.h (working copy) @@ -23,7 +23,6 @@ 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); @@ -47,7 +46,6 @@ gboolean giop_thread_queue_empty_T (GIOPThread tdata); void giop_thread_queue_tail_wakeup(GIOPThread tdata); - #endif /* ORBIT2_INTERNAL_API / G_END_DECLS Index: include/orbit/GIOP/giop-types.h =================================================================== --- include/orbit/GIOP/giop-types.h (revision 2003) +++ include/orbit/GIOP/giop-types.h (working copy) @@ -35,10 +35,8 @@ gpointer dummy); }; -#define GIOP_INITIAL_TIMEOUT_LIMIT (30000000) / 30 seconds / +#define GIOP_INITIAL_MSG_SIZE_LIMIT (2561024) -#define GIOP_INITIAL_MSG_SIZE_LIMIT (2561024) / in bytes / - typedef enum { GIOP_CONNECTION_SSL } GIOPConnectionOptions; Index: include/orbit/GIOP/giop-recv-buffer.h =================================================================== --- include/orbit/GIOP/giop-recv-buffer.h (revision 2003) +++ include/orbit/GIOP/giop-recv-buffer.h (working copy) @@ -74,7 +74,9 @@ void giop_recv_list_zap (GIOPConnection cnx); gboolean giop_connection_handle_input (LinkConnection lcnx); void giop_connection_destroy_frags (GIOPConnection cnx); +extern void giop_timeout_add (GIOPConnection cnx); + #endif / ORBIT2_INTERNAL_API / G_END_DECLS Index: ChangeLog =================================================================== --- ChangeLog (revision 2003) +++ ChangeLog (working copy) @@ -1,3 +1,49 @@ +2007-06-18 Jules Colding <colding@...> + + src/orb/GIOP/giop-send-buffer.c (giop_send_buffer_is_oneway): + New function to tell if the connection is a oneway. + (giop_send_buffer_write): Will add a timeout for the connection + iff the timeout value is non-zero and the connection isn't oneway. + + * src/orb/GIOP/giop-connection.c (giop_set_timeout): New function + to set the link timeout value. + + * src/orb/GIOP/giop-recv-buffer.c (giop_recv_buffer_get): + 1) Handle the LINK_TIMEOUT connection status. + 2) Added new function parameter to communicate the timeout + status to callers. + (giop_timeout_add): New function to add a timeout thread. + (giop_timeout): Timeout thread function. Will wait using nanosleep + until the timeout expires. Will change the link state + to LINK_TIMEOUT and signal wait condition. + + * src/orb/orb-core/corba-object.c (ORBit_try_connection_T): + Handle a timed out link just like a disconnected link. + + * src/orb/orb-core/corba-orb.c: The ORB command line option: + + GIOPTimeoutLimit + + is no more. It has been changed into: + + GIOPTimeoutMSEC - IPv4/6 timeout limit in milliseconds + + It defaults to 60 seconds. This options controls the timeout + limit for GIOP connections, but only for IPv4 and IPv6 + connections. Setting it to zero will explicitly disable any + timeout control. + +2007-06-13 Jules Colding <colding@...> + + * src/orb/util/orbit-options.c (ORBit_option_set): Support parsing + unsigned long option values. + + * src/orb/util/orbit-options.h (enum): Added support for + unsigned long option values. + + * Reverted previous GIOP timeout patch after having discussed + my flawed approach with Michael. + 2007-03-18 Brian Cameron <Brian.Cameron@...> * linc2/src/linc-protocols.c: Now always set the _______________________________________________ orbit-list mailing list orbit-list@... http://mail.gnome.org/mailman/listinfo/orbit-list
	Re: New GIOP timeout patch - please review by Gustavo J. A. M. Carneiro :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message Hi, I must say that launching a thread for each outoging call just to get a timeout is way overkill. The glib main loop, which i'm sure Link is using, already supports timeouts in a much more efficient manner. On Qui, 2007-06-21 at 13:51 +0200, Jules Colding wrote: > Hi all, > > I've reverted my previous timeout approach following well deserved > criticism from Michael. This new approach seems to do the right thing > and I've tested it with a modified echo client/server. Explanation and > patch below. > > Please comment. > > Thanks, > jules > > > > The patch does the following: > > 0) The old timeout patch is reverted. > > 1) The timeout value is initialized in ORBit_ORB_start_servers(). > Explicitly setting the value to 0 (zero) will disable timeout > altogether. The default value is 60 seconds. > > 2) The timeout value is a guint. I've added support for guint ORB > options. > > 3) Timeout helper members has been added to the LinkConnection > structure. This includes a timeout GMutex, a new link enum status member > and the timeout value in milliseconds. > > 4) These timeout members are initialized from there NIL state to > something that will result in an effective timeout on the individual > connection. This happens iff these statements hold true: > > a) The connection is IPv4 or IPv6 > b) The timeout member value is non-zero > > This initialization happens in link_connection_from_fd_T(). > > 5) A timeout thread is effectively launched in giop_send_buffer_write() > but only if the connection is not a oneway. > > 6) The timeout thread function will set the link state to LINK_TIMEOUT > and signal the GCond that giop_recv_buffer_get() waits on. > > 7) ORBit_small_invoke_stub() will push a timeout exception down the line > upon learning from giop_recv_buffer_get() that the connection is timed > out. > > > Index: src/orb/orb-core/corba-orb.c > =================================================================== > --- src/orb/orb-core/corba-orb.c (revision 2003) > +++ src/orb/orb-core/corba-orb.c (working copy) > @@ -61,7 +61,7 @@ > static char orbit_naming_ref = NULL; > static GSList orbit_initref_list = NULL; > static gboolean orbit_use_corbaloc = FALSE; > -static gint orbit_timeout_limit = -1; > +static guint orbit_timeout_msec = 60000; /* 60 seconds - 0 will disable timeouts altogether / > void > ORBit_ORB_start_servers (CORBA_ORB orb) > { > @@ -418,7 +418,7 @@ > #endif / G_ENABLE_DEBUG / > > giop_recv_set_limit (orbit_initial_recv_limit); > - giop_recv_set_timeout (orbit_timeout_limit); > + giop_set_timeout (orbit_timeout_msec); > giop_init (thread_safe, > orbit_use_ipv4 \|\| orbit_use_ipv6 \|\| > orbit_use_irda \|\| orbit_use_ssl); > @@ -1468,7 +1468,7 @@ > { "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 }, > + { "GIOPTimeoutMSEC", ORBIT_OPTION_ULONG, &orbit_timeout_msec }, > { NULL, 0, NULL } > }; > > Index: src/orb/orb-core/corba-object.c > =================================================================== > --- src/orb/orb-core/corba-object.c (revision 2003) > +++ src/orb/orb-core/corba-object.c (working copy) > @@ -270,6 +270,7 @@ > retval = TRUE; > break; > case LINK_DISCONNECTED: > + case LINK_TIMEOUT: > / Have a go at reviving it / > dprintf (MESSAGES, "re-connecting dropped cnx %p: ", cnx); > if (giop_connection_try_reconnect (GIOP_CONNECTION (cnx)) == LINK_CONNECTED) > Index: src/orb/util/orbit-options.c > =================================================================== > --- src/orb/util/orbit-options.c (revision 2003) > +++ src/orb/util/orbit-options.c (working copy) > @@ -53,6 +53,9 @@ > case ORBIT_OPTION_INT: > (gint )option->arg = atoi (val); > break; > + case ORBIT_OPTION_ULONG: > + (guint )option->arg = strtoul(val, (char )NULL, 10); > + break; > case ORBIT_OPTION_STRING: { > gchar str_arg = (char ) option->arg; > > Index: src/orb/util/orbit-options.h > =================================================================== > --- src/orb/util/orbit-options.h (revision 2003) > +++ src/orb/util/orbit-options.h (working copy) > @@ -10,7 +10,8 @@ > ORBIT_OPTION_STRING, > ORBIT_OPTION_INT, > ORBIT_OPTION_BOOLEAN, > - ORBIT_OPTION_KEY_VALUE / returns GSList of ORBit_option_key_value / > + ORBIT_OPTION_KEY_VALUE, / returns GSList of ORBit_option_key_value / > + ORBIT_OPTION_ULONG, > } ORBit_option_type; > > typedef struct { > Index: src/orb/GIOP/giop-recv-buffer.c > =================================================================== > --- src/orb/GIOP/giop-recv-buffer.c (revision 2003) > +++ src/orb/GIOP/giop-recv-buffer.c (working copy) > @@ -686,26 +686,20 @@ > static inline gboolean > check_got (GIOPMessageQueueEntry ent) > { > - return (ent->buffer \|\| !ent->cnx \|\| > - (ent->cnx->parent.status == LINK_DISCONNECTED)); > + return (ent->buffer \|\| > + !ent->cnx \|\| > + (ent->cnx->parent.status == LINK_DISCONNECTED) \|\| > + (ent->cnx->parent.status == LINK_TIMEOUT)); > } > > -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, > gboolean timeout) > { > GIOPThread tdata = giop_thread_self (); > - GTimeVal tval; > > + timeout = FALSE; > + > thread_switch: > if (giop_thread_io ()) { > ent_lock (ent); > @@ -715,17 +709,8 @@ > ent_unlock (ent); > giop_thread_queue_process (tdata); > ent_lock (ent); > - } 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; > - } > + } else > + g_cond_wait (tdata->incoming, tdata->lock); > } > > ent_unlock (ent); > @@ -734,6 +719,7 @@ > > while (!ent->buffer && ent->cnx && > (ent->cnx->parent.status != LINK_DISCONNECTED) && > + (ent->cnx->parent.status != LINK_TIMEOUT) && > !giop_thread_io()) > link_main_iteration (TRUE); > > @@ -741,6 +727,13 @@ > goto thread_switch; > } > > + g_mutex_lock (ent->cnx->parent.timeout_mutex); > + if (ent->cnx->parent.timeout_status == LINK_TIMEOUT_UNKNOWN) > + ent->cnx->parent.timeout_status = LINK_TIMEOUT_NO; > + else > + timeout = TRUE; > + g_mutex_unlock (ent->cnx->parent.timeout_mutex); > + > giop_thread_queue_tail_wakeup (tdata); > giop_recv_list_destroy_queue_entry (ent); > > @@ -1355,6 +1348,73 @@ > return TRUE; > } > > +struct timeout_thread_data { > + guint msec; > + GIOPThread tdata; > + LinkConnection lcnx; > +}; > + > +static gpointer > +giop_timeout(gpointer data) > +{ > + guint msec = ((struct timeout_thread_data)data)->msec; > + LinkConnection lcnx = ((struct timeout_thread_data)data)->lcnx; > + GIOPThread tdata = ((struct timeout_thread_data)data)->tdata; > + struct timespec tv; > + > + g_assert (lcnx->timeout_mutex); > + > + tv.tv_sec = msec / 1000; > + tv.tv_nsec = (msec - tv.tv_sec1000) 1000000; > + > + nanosleep (&tv, NULL); > + > + g_mutex_lock (lcnx->timeout_mutex); > + if (lcnx->timeout_status == LINK_TIMEOUT_UNKNOWN) > + lcnx->timeout_status = LINK_TIMEOUT_YES; > + else { > + g_mutex_unlock (lcnx->timeout_mutex); > + goto out; > + } > + g_mutex_unlock (lcnx->timeout_mutex); > + > + link_connection_state_changed (lcnx, LINK_TIMEOUT); > + > + g_mutex_lock (tdata->lock); /* ent_lock / > + giop_incoming_signal_T (tdata, GIOP_CLOSECONNECTION); > + g_mutex_unlock (tdata->lock); / ent_lock / > + > +out: > + g_object_unref (lcnx); > + g_free (data); > + > + return NULL; > +} > + > +void > +giop_timeout_add(GIOPConnection cnx) > +{ > + struct timeout_thread_data data = NULL; > + LinkConnection lcnx = LINK_CONNECTION (cnx); > + > + if (!lcnx->timeout_msec) > + return; > + > + g_object_ref (lcnx); > + > + lcnx->timeout_mutex = g_mutex_new (); > + > + data = g_new0 (struct timeout_thread_data, 1); > + data->msec = lcnx->timeout_msec; > + data->tdata = giop_thread_self (); > + data->lcnx = lcnx; > + > + g_thread_create (giop_timeout, > + (gpointer)data, > + FALSE, > + NULL); > +} > + > GIOPRecvBuffer * > giop_recv_buffer_use_buf (GIOPConnection cnx) > { > @@ -1372,4 +1432,3 @@ > > return buf; > } > - > Index: src/orb/GIOP/giop-connection.c > =================================================================== > --- src/orb/GIOP/giop-connection.c (revision 2003) > +++ src/orb/GIOP/giop-connection.c (working copy) > @@ -108,6 +108,7 @@ > } > } > > + > static void > giop_connection_class_init (GIOPConnectionClass klass) > { > @@ -192,3 +193,9 @@ > return link_connection_try_reconnect (LINK_CONNECTION (cnx)); > } > > +void > +giop_set_timeout (guint msec) > +{ > + link_set_timeout (msec); > +} > + > Index: src/orb/GIOP/giop-send-buffer.c > =================================================================== > --- src/orb/GIOP/giop-send-buffer.c (revision 2003) > +++ src/orb/GIOP/giop-send-buffer.c (working copy) > @@ -45,6 +45,25 @@ > > static const GIOP_AddressingDisposition giop_1_2_target_type = GIOP_KeyAddr; > > +static gboolean > +giop_send_buffer_is_oneway(const GIOPSendBuffer buf) > +{ > + g_assert (buf); > + > + switch (buf->giop_version) { > + case GIOP_1_0: > + case GIOP_1_1: > + return (buf->msg.u.request_1_0.response_expected ? FALSE : TRUE); > + case GIOP_1_2: > + return (buf->msg.u.request_1_2.response_flags ? FALSE : TRUE); > + default: > + break; > + } > + g_assert_not_reached(); > + > + return TRUE; > +} > + > GIOPSendBuffer > giop_send_buffer_use_request (GIOPVersion giop_version, > CORBA_unsigned_long request_id, > @@ -426,6 +445,7 @@ > gboolean blocking) > { > int retval; > + LinkConnection lcnx = LINK_CONNECTION (cnx); > static LinkWriteOpts non_block = NULL; > > if (!non_block) > @@ -434,11 +454,15 @@ > /* FIXME: if a FRAGMENT, assert the 8 byte tail align, > &&\|\| giop_send_buffer_align (buf, 8); / > > - retval = link_connection_writev ( > - (LinkConnection ) cnx, buf->iovecs, > - buf->num_used, > - blocking ? NULL : non_block); > + if (lcnx->timeout_msec && !giop_send_buffer_is_oneway (buf)) { > + giop_timeout_add (cnx); > + } > > + retval = link_connection_writev (lcnx, > + buf->iovecs, > + buf->num_used, > + blocking ? NULL : non_block); > + > if (!blocking && retval == LINK_IO_QUEUED_DATA) > retval = 0; > > Index: linc2/include/linc/linc-connection.h > =================================================================== > --- linc2/include/linc/linc-connection.h (revision 2003) > +++ linc2/include/linc/linc-connection.h (working copy) > @@ -39,7 +39,8 @@ > #define LINK_IS_CONNECTION(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), LINK_TYPE_CONNECTION)) > #define LINK_IS_CONNECTION_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), LINK_TYPE_CONNECTION)) > > -typedef enum { LINK_CONNECTING, LINK_CONNECTED, LINK_DISCONNECTED } LinkConnectionStatus; > +typedef enum { LINK_CONNECTING, LINK_CONNECTED, LINK_DISCONNECTED, LINK_TIMEOUT } LinkConnectionStatus; > +typedef enum { LINK_TIMEOUT_UNKNOWN, LINK_TIMEOUT_YES, LINK_TIMEOUT_NO } LinkTimeoutStatus; > > typedef struct _LinkWriteOpts LinkWriteOpts; > typedef struct _LinkConnectionPrivate LinkConnectionPrivate; > @@ -61,6 +62,10 @@ > LinkConnectionPrivate priv; > > GSList idle_broken_callbacks; > + > + GMutex timeout_mutex; > + guint timeout_msec; > + LinkTimeoutStatus timeout_status;; > } LinkConnection; > > typedef struct { > @@ -153,6 +158,9 @@ > > void link_connections_close (void); > > +/ set the link timeout in miliseconds / > +extern void link_set_timeout (guint msec); > + > G_END_DECLS > > #endif / _LINK_CONNECTION_H / > Index: linc2/include/linc/linc.h > =================================================================== > --- linc2/include/linc/linc.h (revision 2003) > +++ linc2/include/linc/linc.h (working copy) > @@ -33,7 +33,7 @@ > guint link_main_idle_add (GSourceFunc function, > gpointer data); > > -gboolean link_wait (void); > +void link_wait (void); > void link_signal (void); > > gboolean link_thread_io (void); > Index: linc2/ChangeLog > =================================================================== > --- linc2/ChangeLog (revision 2003) > +++ linc2/ChangeLog (working copy) > @@ -1,3 +1,24 @@ > +2007-06-19 Jules Colding <colding@...> > + > + src/linc-connection.c (link_connection_init): Initialize > + timeout members in the LinkConnection structure. > + (link_set_timeout): New function to set the timeout value. > + > +2007-06-18 Jules Colding <colding@...> > + > + * src/linc-connection.c (link_connection_from_fd_T): Initiate > + timeout members of the link connection iff: > + 1) The connection is IPv4 or IPv6 > + 2) It is not a oneway > + 3) The timeout parameter is non-zero > + > + * include/linc/linc-connection.h (struct): > + 1) timeout mutex > + 2) timeout value in milliseconds > + 3) timeout status > + > + Furthermore declare link_set_timeout() > + > ========================== ORBit2-2.14.8 ======================== > > 2007-02-27 Kjartan Maraas <kmaraas@...> > Index: linc2/src/linc-debug.h > =================================================================== > --- linc2/src/linc-debug.h (revision 2003) > +++ linc2/src/linc-debug.h (working copy) > @@ -25,6 +25,7 @@ > # define STATE_NAME(s) (((s) == LINK_CONNECTED) ? "Connected" : \ > ((s) == LINK_CONNECTING) ? "Connecting" : \ > ((s) == LINK_DISCONNECTED) ? "Disconnected" : \ > + ((s) == LINK_TIMEOUT) ? "Timeout" : \ > "Invalid state") > # ifdef CONNECTION_DEBUG_FLAG > extern gboolean link_connection_debug_flag; > Index: linc2/src/linc-connection.c > =================================================================== > --- linc2/src/linc-connection.c (revision 2003) > +++ linc2/src/linc-connection.c (working copy) > @@ -27,6 +27,7 @@ > #include <linc/linc-connection.h> > > static GObjectClass parent_class = NULL; > +static guint _link_timeout = 0; > > enum { > BROKEN, > @@ -288,6 +289,7 @@ > break; > > case LINK_DISCONNECTED: > + case LINK_TIMEOUT: > link_source_remove (cnx); > link_close_fd (cnx); > queue_free (cnx); > @@ -440,6 +442,19 @@ > g_free (cnx->remote_serv_info); > cnx->remote_serv_info = remote_serv_info; > > + switch (cnx->proto->family) { > + case AF_INET: > + case AF_INET6: > + if (_link_timeout && !cnx->timeout_msec) { / this should'nt happen twice but I'm always paranoid... / > + cnx->timeout_mutex = NULL; > + cnx->timeout_msec = _link_timeout; > + cnx->timeout_status = LINK_TIMEOUT_UNKNOWN; > + } > + break; > + default: > + break; > + } > + > d_printf ("Cnx from fd (%d) '%s', '%s', '%s'\n", > fd, proto->name, > remote_host_info ? remote_host_info : "<Null>", > @@ -635,10 +650,8 @@ > static LinkConnectionStatus > link_connection_wait_connected_T (LinkConnection cnx) > { > - while (cnx && cnx->status == LINK_CONNECTING) { > - if (!link_wait ()) > - link_connection_disconnect (cnx); > - } > + while (cnx && cnx->status == LINK_CONNECTING) > + link_wait (); > > return cnx ? cnx->status : LINK_DISCONNECTED; > } > @@ -661,12 +674,8 @@ > cnx->inhibit_reconnect = FALSE; > dispatch_callbacks_drop_lock (cnx); > g_main_context_release (NULL); > - } else { > - if (!link_wait ()) { > - link_connection_disconnect (cnx); > - break; > - } > - } > + } else > + link_wait (); > } > > if (cnx->status != LINK_DISCONNECTED) > @@ -1254,6 +1263,9 @@ > > g_free (cnx->priv); > > + if (cnx->timeout_mutex) > + g_mutex_free (cnx->timeout_mutex); > + > #ifdef G_ENABLE_DEBUG > g_assert (g_list_find(cnx_list, cnx) == NULL); > #endif > @@ -1269,6 +1281,11 @@ > cnx->priv = g_new0 (LinkConnectionPrivate, 1); > cnx->priv->fd = -1; > cnx->priv->was_disconnected = FALSE; > + > + cnx->timeout_mutex = NULL; > + cnx->timeout_msec = 0; > + cnx->timeout_status = LINK_TIMEOUT_UNKNOWN; > + > #ifdef CONNECTION_DEBUG > cnx->priv->total_read_bytes = 0; > cnx->priv->total_written_bytes = 0; > @@ -1568,3 +1585,10 @@ > > g_list_free (cnx); > } > + > +void > +link_set_timeout (guint msec) > +{ > + _link_timeout = msec; > +} > + > Index: linc2/src/linc.c > =================================================================== > --- linc2/src/linc.c (revision 2003) > +++ linc2/src/linc.c (working copy) > @@ -52,9 +52,6 @@ > 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 > @@ -537,28 +534,17 @@ > } > } > > -gboolean > +void > 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_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; > - } > + g_cond_wait (link_main_cond, link_main_lock); > } > - > - return TRUE; > } > > > Index: include/orbit/GIOP/giop-connection.h > =================================================================== > --- include/orbit/GIOP/giop-connection.h (revision 2003) > +++ include/orbit/GIOP/giop-connection.h (working copy) > @@ -53,6 +53,9 @@ > #define giop_connection_ref(cnx) link_connection_ref(cnx) > #define giop_connection_unref(cnx) link_connection_unref(cnx) > > +/ set the link timeout in milliseconds / > +extern void giop_set_timeout (guint msec); > + > #endif / ORBIT2_INTERNAL_API / > > G_END_DECLS > Index: include/orbit/GIOP/giop.h > =================================================================== > --- include/orbit/GIOP/giop.h (revision 2003) > +++ include/orbit/GIOP/giop.h (working copy) > @@ -23,7 +23,6 @@ > 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); > @@ -47,7 +46,6 @@ > gboolean giop_thread_queue_empty_T (GIOPThread tdata); > void giop_thread_queue_tail_wakeup(GIOPThread tdata); > > - > #endif /* ORBIT2_INTERNAL_API / > > G_END_DECLS > Index: include/orbit/GIOP/giop-types.h > =================================================================== > --- include/orbit/GIOP/giop-types.h (revision 2003) > +++ include/orbit/GIOP/giop-types.h (working copy) > @@ -35,10 +35,8 @@ > gpointer dummy); > }; > > -#define GIOP_INITIAL_TIMEOUT_LIMIT (30000000) / 30 seconds / > +#define GIOP_INITIAL_MSG_SIZE_LIMIT (2561024) > > -#define GIOP_INITIAL_MSG_SIZE_LIMIT (2561024) / in bytes / > - > typedef enum { > GIOP_CONNECTION_SSL > } GIOPConnectionOptions; > Index: include/orbit/GIOP/giop-recv-buffer.h > =================================================================== > --- include/orbit/GIOP/giop-recv-buffer.h (revision 2003) > +++ include/orbit/GIOP/giop-recv-buffer.h (working copy) > @@ -74,7 +74,9 @@ > void giop_recv_list_zap (GIOPConnection cnx); > gboolean giop_connection_handle_input (LinkConnection lcnx); > void giop_connection_destroy_frags (GIOPConnection cnx); > +extern void giop_timeout_add (GIOPConnection cnx); > > + > #endif / ORBIT2_INTERNAL_API / > > G_END_DECLS > Index: ChangeLog > =================================================================== > --- ChangeLog (revision 2003) > +++ ChangeLog (working copy) > @@ -1,3 +1,49 @@ > +2007-06-18 Jules Colding <colding@...> > + > + src/orb/GIOP/giop-send-buffer.c (giop_send_buffer_is_oneway): > + New function to tell if the connection is a oneway. > + (giop_send_buffer_write): Will add a timeout for the connection > + iff the timeout value is non-zero and the connection isn't oneway. > + > + * src/orb/GIOP/giop-connection.c (giop_set_timeout): New function > + to set the link timeout value. > + > + * src/orb/GIOP/giop-recv-buffer.c (giop_recv_buffer_get): > + 1) Handle the LINK_TIMEOUT connection status. > + 2) Added new function parameter to communicate the timeout > + status to callers. > + (giop_timeout_add): New function to add a timeout thread. > + (giop_timeout): Timeout thread function. Will wait using nanosleep > + until the timeout expires. Will change the link state > + to LINK_TIMEOUT and signal wait condition. > + > + * src/orb/orb-core/corba-object.c (ORBit_try_connection_T): > + Handle a timed out link just like a disconnected link. > + > + * src/orb/orb-core/corba-orb.c: The ORB command line option: > + > + GIOPTimeoutLimit > + > + is no more. It has been changed into: > + > + GIOPTimeoutMSEC - IPv4/6 timeout limit in milliseconds > + > + It defaults to 60 seconds. This options controls the timeout > + limit for GIOP connections, but only for IPv4 and IPv6 > + connections. Setting it to zero will explicitly disable any > + timeout control. > + > +2007-06-13 Jules Colding <colding@...> > + > + * src/orb/util/orbit-options.c (ORBit_option_set): Support parsing > + unsigned long option values. > + > + * src/orb/util/orbit-options.h (enum): Added support for > + unsigned long option values. > + > + * Reverted previous GIOP timeout patch after having discussed > + my flawed approach with Michael. > + > 2007-03-18 Brian Cameron <Brian.Cameron@...> > > * linc2/src/linc-protocols.c: Now always set the > > > _______________________________________________ > orbit-list mailing list > orbit-list@... > http://mail.gnome.org/mailman/listinfo/orbit-list -- Gustavo J. A. M. Carneiro <gjc@...> <gustavo@...> "The universe is always one step beyond logic" -- Frank Herbert _______________________________________________ orbit-list mailing list orbit-list@... http://mail.gnome.org/mailman/listinfo/orbit-list
	Re: New GIOP timeout patch - please review by colding :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message On Thu, 2007-06-21 at 13:09 +0100, Gustavo J. A. M. Carneiro wrote: > Hi, > > I must say that launching a thread for each outoging call just to get > a timeout is way overkill. The glib main loop, which i'm sure Link is > using, already supports timeouts in a much more efficient manner. I've already tried that with no success. Not sure why it didn't work, but my first set of patches used this approach. The timeout failed to trigger. -- jules _______________________________________________ orbit-list mailing list orbit-list@... http://mail.gnome.org/mailman/listinfo/orbit-list
	Re: New GIOP timeout patch - please review by colding :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message On Thu, 2007-06-21 at 13:51 +0200, Jules Colding wrote: > Hi all, > > I've reverted my previous timeout approach following well deserved > criticism from Michael. This new approach seems to do the right thing > and I've tested it with a modified echo client/server. Explanation and > patch below. > > Please comment. As always... Just noticed two small and stupid bugs. Corrected patch below. Sorry, jules Index: src/orb/orb-core/corba-orb.c =================================================================== --- src/orb/orb-core/corba-orb.c (revision 2003) +++ src/orb/orb-core/corba-orb.c (working copy) @@ -61,7 +61,7 @@ static char orbit_naming_ref = NULL; static GSList orbit_initref_list = NULL; static gboolean orbit_use_corbaloc = FALSE; -static gint orbit_timeout_limit = -1; +static guint orbit_timeout_msec = 60000; /* 60 seconds - 0 will disable timeouts altogether / void ORBit_ORB_start_servers (CORBA_ORB orb) { @@ -418,7 +418,7 @@ #endif / G_ENABLE_DEBUG / giop_recv_set_limit (orbit_initial_recv_limit); - giop_recv_set_timeout (orbit_timeout_limit); + giop_set_timeout (orbit_timeout_msec); giop_init (thread_safe, orbit_use_ipv4 \|\| orbit_use_ipv6 \|\| orbit_use_irda \|\| orbit_use_ssl); @@ -1468,7 +1468,7 @@ { "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 }, + { "GIOPTimeoutMSEC", ORBIT_OPTION_ULONG, &orbit_timeout_msec }, { NULL, 0, NULL } }; Index: src/orb/orb-core/corba-object.c =================================================================== --- src/orb/orb-core/corba-object.c (revision 2003) +++ src/orb/orb-core/corba-object.c (working copy) @@ -270,6 +270,7 @@ retval = TRUE; break; case LINK_DISCONNECTED: + case LINK_TIMEOUT: / Have a go at reviving it / dprintf (MESSAGES, "re-connecting dropped cnx %p: ", cnx); if (giop_connection_try_reconnect (GIOP_CONNECTION (cnx)) == LINK_CONNECTED) Index: src/orb/util/orbit-options.c =================================================================== --- src/orb/util/orbit-options.c (revision 2003) +++ src/orb/util/orbit-options.c (working copy) @@ -53,6 +53,9 @@ case ORBIT_OPTION_INT: (gint )option->arg = atoi (val); break; + case ORBIT_OPTION_ULONG: + (guint )option->arg = strtoul(val, (char )NULL, 10); + break; case ORBIT_OPTION_STRING: { gchar str_arg = (char ) option->arg; Index: src/orb/util/orbit-options.h =================================================================== --- src/orb/util/orbit-options.h (revision 2003) +++ src/orb/util/orbit-options.h (working copy) @@ -10,7 +10,8 @@ ORBIT_OPTION_STRING, ORBIT_OPTION_INT, ORBIT_OPTION_BOOLEAN, - ORBIT_OPTION_KEY_VALUE / returns GSList of ORBit_option_key_value / + ORBIT_OPTION_KEY_VALUE, / returns GSList of ORBit_option_key_value / + ORBIT_OPTION_ULONG, } ORBit_option_type; typedef struct { Index: src/orb/GIOP/giop-recv-buffer.c =================================================================== --- src/orb/GIOP/giop-recv-buffer.c (revision 2003) +++ src/orb/GIOP/giop-recv-buffer.c (working copy) @@ -686,26 +686,20 @@ static inline gboolean check_got (GIOPMessageQueueEntry ent) { - return (ent->buffer \|\| !ent->cnx \|\| - (ent->cnx->parent.status == LINK_DISCONNECTED)); + return (ent->buffer \|\| + !ent->cnx \|\| + (ent->cnx->parent.status == LINK_DISCONNECTED) \|\| + (ent->cnx->parent.status == LINK_TIMEOUT)); } -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, gboolean timeout) { GIOPThread tdata = giop_thread_self (); - GTimeVal tval; + timeout = FALSE; + thread_switch: if (giop_thread_io ()) { ent_lock (ent); @@ -715,17 +709,8 @@ ent_unlock (ent); giop_thread_queue_process (tdata); ent_lock (ent); - } 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; - } + } else + g_cond_wait (tdata->incoming, tdata->lock); } ent_unlock (ent); @@ -734,6 +719,7 @@ while (!ent->buffer && ent->cnx && (ent->cnx->parent.status != LINK_DISCONNECTED) && + (ent->cnx->parent.status != LINK_TIMEOUT) && !giop_thread_io()) link_main_iteration (TRUE); @@ -741,6 +727,15 @@ goto thread_switch; } + if (ent->cnx->parent.timeout_mutex) { + g_mutex_lock (ent->cnx->parent.timeout_mutex); + if (ent->cnx->parent.timeout_status == LINK_TIMEOUT_UNKNOWN) + ent->cnx->parent.timeout_status = LINK_TIMEOUT_NO; + else + timeout = TRUE; + g_mutex_unlock (ent->cnx->parent.timeout_mutex); + } + giop_thread_queue_tail_wakeup (tdata); giop_recv_list_destroy_queue_entry (ent); @@ -1355,6 +1350,74 @@ return TRUE; } +struct timeout_thread_data { + guint msec; + GIOPThread tdata; + LinkConnection lcnx; +}; + +static gpointer +giop_timeout(gpointer data) +{ + guint msec = ((struct timeout_thread_data)data)->msec; + LinkConnection lcnx = ((struct timeout_thread_data)data)->lcnx; + GIOPThread tdata = ((struct timeout_thread_data)data)->tdata; + struct timespec tv; + + g_assert (lcnx->timeout_mutex); + + tv.tv_sec = msec / 1000; + tv.tv_nsec = (msec - tv.tv_sec1000) 1000000; + + nanosleep (&tv, NULL); + + g_mutex_lock (lcnx->timeout_mutex); + if (lcnx->timeout_status == LINK_TIMEOUT_UNKNOWN) + lcnx->timeout_status = LINK_TIMEOUT_YES; + else { + g_mutex_unlock (lcnx->timeout_mutex); + goto out; + } + g_mutex_unlock (lcnx->timeout_mutex); + + link_connection_state_changed (lcnx, LINK_TIMEOUT); + + g_mutex_lock (tdata->lock); /* ent_lock / + giop_incoming_signal_T (tdata, GIOP_CLOSECONNECTION); + g_mutex_unlock (tdata->lock); / ent_lock / + +out: + g_object_unref (lcnx); + g_free (data); + + return NULL; +} + +void +giop_timeout_add(GIOPConnection cnx) +{ + struct timeout_thread_data data = NULL; + LinkConnection lcnx = LINK_CONNECTION (cnx); + + if (!lcnx->timeout_msec) + return; + + g_object_ref (lcnx); + + if (!lcnx->timeout_mutex) + lcnx->timeout_mutex = g_mutex_new (); + + data = g_new0 (struct timeout_thread_data, 1); + data->msec = lcnx->timeout_msec; + data->tdata = giop_thread_self (); + data->lcnx = lcnx; + + g_thread_create (giop_timeout, + (gpointer)data, + FALSE, + NULL); +} + GIOPRecvBuffer * giop_recv_buffer_use_buf (GIOPConnection cnx) { @@ -1372,4 +1435,3 @@ return buf; } - Index: src/orb/GIOP/giop-connection.c =================================================================== --- src/orb/GIOP/giop-connection.c (revision 2003) +++ src/orb/GIOP/giop-connection.c (working copy) @@ -108,6 +108,7 @@ } } + static void giop_connection_class_init (GIOPConnectionClass klass) { @@ -192,3 +193,9 @@ return link_connection_try_reconnect (LINK_CONNECTION (cnx)); } +void +giop_set_timeout (guint msec) +{ + link_set_timeout (msec); +} + Index: src/orb/GIOP/giop-send-buffer.c =================================================================== --- src/orb/GIOP/giop-send-buffer.c (revision 2003) +++ src/orb/GIOP/giop-send-buffer.c (working copy) @@ -45,6 +45,25 @@ static const GIOP_AddressingDisposition giop_1_2_target_type = GIOP_KeyAddr; +static gboolean +giop_send_buffer_is_oneway(const GIOPSendBuffer buf) +{ + g_assert (buf); + + switch (buf->giop_version) { + case GIOP_1_0: + case GIOP_1_1: + return (buf->msg.u.request_1_0.response_expected ? FALSE : TRUE); + case GIOP_1_2: + return (buf->msg.u.request_1_2.response_flags ? FALSE : TRUE); + default: + break; + } + g_assert_not_reached(); + + return TRUE; +} + GIOPSendBuffer giop_send_buffer_use_request (GIOPVersion giop_version, CORBA_unsigned_long request_id, @@ -426,6 +445,7 @@ gboolean blocking) { int retval; + LinkConnection lcnx = LINK_CONNECTION (cnx); static LinkWriteOpts non_block = NULL; if (!non_block) @@ -434,11 +454,15 @@ /* FIXME: if a FRAGMENT, assert the 8 byte tail align, &&\|\| giop_send_buffer_align (buf, 8); / - retval = link_connection_writev ( - (LinkConnection ) cnx, buf->iovecs, - buf->num_used, - blocking ? NULL : non_block); + if (lcnx->timeout_msec && !giop_send_buffer_is_oneway (buf)) { + giop_timeout_add (cnx); + } + retval = link_connection_writev (lcnx, + buf->iovecs, + buf->num_used, + blocking ? NULL : non_block); + if (!blocking && retval == LINK_IO_QUEUED_DATA) retval = 0; Index: linc2/include/linc/linc-connection.h =================================================================== --- linc2/include/linc/linc-connection.h (revision 2003) +++ linc2/include/linc/linc-connection.h (working copy) @@ -39,7 +39,8 @@ #define LINK_IS_CONNECTION(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), LINK_TYPE_CONNECTION)) #define LINK_IS_CONNECTION_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), LINK_TYPE_CONNECTION)) -typedef enum { LINK_CONNECTING, LINK_CONNECTED, LINK_DISCONNECTED } LinkConnectionStatus; +typedef enum { LINK_CONNECTING, LINK_CONNECTED, LINK_DISCONNECTED, LINK_TIMEOUT } LinkConnectionStatus; +typedef enum { LINK_TIMEOUT_UNKNOWN, LINK_TIMEOUT_YES, LINK_TIMEOUT_NO } LinkTimeoutStatus; typedef struct _LinkWriteOpts LinkWriteOpts; typedef struct _LinkConnectionPrivate LinkConnectionPrivate; @@ -61,6 +62,10 @@ LinkConnectionPrivate priv; GSList idle_broken_callbacks; + + GMutex timeout_mutex; + guint timeout_msec; + LinkTimeoutStatus timeout_status;; } LinkConnection; typedef struct { @@ -153,6 +158,9 @@ void link_connections_close (void); +/ set the link timeout in miliseconds / +extern void link_set_timeout (guint msec); + G_END_DECLS #endif / _LINK_CONNECTION_H / Index: linc2/include/linc/linc.h =================================================================== --- linc2/include/linc/linc.h (revision 2003) +++ linc2/include/linc/linc.h (working copy) @@ -33,7 +33,7 @@ guint link_main_idle_add (GSourceFunc function, gpointer data); -gboolean link_wait (void); +void link_wait (void); void link_signal (void); gboolean link_thread_io (void); Index: linc2/ChangeLog =================================================================== --- linc2/ChangeLog (revision 2003) +++ linc2/ChangeLog (working copy) @@ -1,3 +1,24 @@ +2007-06-19 Jules Colding <colding@...> + + src/linc-connection.c (link_connection_init): Initialize + timeout members in the LinkConnection structure. + (link_set_timeout): New function to set the timeout value. + +2007-06-18 Jules Colding <colding@...> + + * src/linc-connection.c (link_connection_from_fd_T): Initiate + timeout members of the link connection iff: + 1) The connection is IPv4 or IPv6 + 2) It is not a oneway + 3) The timeout parameter is non-zero + + * include/linc/linc-connection.h (struct): + 1) timeout mutex + 2) timeout value in milliseconds + 3) timeout status + + Furthermore declare link_set_timeout() + ========================== ORBit2-2.14.8 ======================== 2007-02-27 Kjartan Maraas <kmaraas@...> Index: linc2/src/linc-debug.h =================================================================== --- linc2/src/linc-debug.h (revision 2003) +++ linc2/src/linc-debug.h (working copy) @@ -25,6 +25,7 @@ # define STATE_NAME(s) (((s) == LINK_CONNECTED) ? "Connected" : \ ((s) == LINK_CONNECTING) ? "Connecting" : \ ((s) == LINK_DISCONNECTED) ? "Disconnected" : \ + ((s) == LINK_TIMEOUT) ? "Timeout" : \ "Invalid state") # ifdef CONNECTION_DEBUG_FLAG extern gboolean link_connection_debug_flag; Index: linc2/src/linc-connection.c =================================================================== --- linc2/src/linc-connection.c (revision 2003) +++ linc2/src/linc-connection.c (working copy) @@ -27,6 +27,7 @@ #include <linc/linc-connection.h> static GObjectClass parent_class = NULL; +static guint _link_timeout = 0; enum { BROKEN, @@ -288,6 +289,7 @@ break; case LINK_DISCONNECTED: + case LINK_TIMEOUT: link_source_remove (cnx); link_close_fd (cnx); queue_free (cnx); @@ -440,6 +442,16 @@ g_free (cnx->remote_serv_info); cnx->remote_serv_info = remote_serv_info; + switch (cnx->proto->family) { + case AF_INET: + case AF_INET6: + if (_link_timeout && !cnx->timeout_msec) / this should'nt happen twice but I'm always paranoid... / + cnx->timeout_msec = _link_timeout; + break; + default: + break; + } + d_printf ("Cnx from fd (%d) '%s', '%s', '%s'\n", fd, proto->name, remote_host_info ? remote_host_info : "<Null>", @@ -635,10 +647,8 @@ static LinkConnectionStatus link_connection_wait_connected_T (LinkConnection cnx) { - while (cnx && cnx->status == LINK_CONNECTING) { - if (!link_wait ()) - link_connection_disconnect (cnx); - } + while (cnx && cnx->status == LINK_CONNECTING) + link_wait (); return cnx ? cnx->status : LINK_DISCONNECTED; } @@ -661,12 +671,8 @@ cnx->inhibit_reconnect = FALSE; dispatch_callbacks_drop_lock (cnx); g_main_context_release (NULL); - } else { - if (!link_wait ()) { - link_connection_disconnect (cnx); - break; - } - } + } else + link_wait (); } if (cnx->status != LINK_DISCONNECTED) @@ -1254,6 +1260,9 @@ g_free (cnx->priv); + if (cnx->timeout_mutex) + g_mutex_free (cnx->timeout_mutex); + #ifdef G_ENABLE_DEBUG g_assert (g_list_find(cnx_list, cnx) == NULL); #endif @@ -1269,6 +1278,11 @@ cnx->priv = g_new0 (LinkConnectionPrivate, 1); cnx->priv->fd = -1; cnx->priv->was_disconnected = FALSE; + + cnx->timeout_mutex = NULL; + cnx->timeout_msec = 0; + cnx->timeout_status = LINK_TIMEOUT_UNKNOWN; + #ifdef CONNECTION_DEBUG cnx->priv->total_read_bytes = 0; cnx->priv->total_written_bytes = 0; @@ -1568,3 +1582,10 @@ g_list_free (cnx); } + +void +link_set_timeout (guint msec) +{ + _link_timeout = msec; +} + Index: linc2/src/linc.c =================================================================== --- linc2/src/linc.c (revision 2003) +++ linc2/src/linc.c (working copy) @@ -52,9 +52,6 @@ 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 @@ -537,28 +534,17 @@ } } -gboolean +void 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_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; - } + g_cond_wait (link_main_cond, link_main_lock); } - - return TRUE; } Index: include/orbit/GIOP/giop-connection.h =================================================================== --- include/orbit/GIOP/giop-connection.h (revision 2003) +++ include/orbit/GIOP/giop-connection.h (working copy) @@ -53,6 +53,9 @@ #define giop_connection_ref(cnx) link_connection_ref(cnx) #define giop_connection_unref(cnx) link_connection_unref(cnx) +/ set the link timeout in milliseconds / +extern void giop_set_timeout (guint msec); + #endif / ORBIT2_INTERNAL_API / G_END_DECLS Index: include/orbit/GIOP/giop.h =================================================================== --- include/orbit/GIOP/giop.h (revision 2003) +++ include/orbit/GIOP/giop.h (working copy) @@ -23,7 +23,6 @@ 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); @@ -47,7 +46,6 @@ gboolean giop_thread_queue_empty_T (GIOPThread tdata); void giop_thread_queue_tail_wakeup(GIOPThread tdata); - #endif /* ORBIT2_INTERNAL_API / G_END_DECLS Index: include/orbit/GIOP/giop-types.h =================================================================== --- include/orbit/GIOP/giop-types.h (revision 2003) +++ include/orbit/GIOP/giop-types.h (working copy) @@ -35,10 +35,8 @@ gpointer dummy); }; -#define GIOP_INITIAL_TIMEOUT_LIMIT (30000000) / 30 seconds / +#define GIOP_INITIAL_MSG_SIZE_LIMIT (2561024) -#define GIOP_INITIAL_MSG_SIZE_LIMIT (2561024) / in bytes / - typedef enum { GIOP_CONNECTION_SSL } GIOPConnectionOptions; Index: include/orbit/GIOP/giop-recv-buffer.h =================================================================== --- include/orbit/GIOP/giop-recv-buffer.h (revision 2003) +++ include/orbit/GIOP/giop-recv-buffer.h (working copy) @@ -74,7 +74,9 @@ void giop_recv_list_zap (GIOPConnection cnx); gboolean giop_connection_handle_input (LinkConnection lcnx); void giop_connection_destroy_frags (GIOPConnection cnx); +extern void giop_timeout_add (GIOPConnection cnx); + #endif / ORBIT2_INTERNAL_API / G_END_DECLS Index: ChangeLog =================================================================== --- ChangeLog (revision 2003) +++ ChangeLog (working copy) @@ -1,3 +1,49 @@ +2007-06-18 Jules Colding <colding@...> + + src/orb/GIOP/giop-send-buffer.c (giop_send_buffer_is_oneway): + New function to tell if the connection is a oneway. + (giop_send_buffer_write): Will add a timeout for the connection + iff the timeout value is non-zero and the connection isn't oneway. + + * src/orb/GIOP/giop-connection.c (giop_set_timeout): New function + to set the link timeout value. + + * src/orb/GIOP/giop-recv-buffer.c (giop_recv_buffer_get): + 1) Handle the LINK_TIMEOUT connection status. + 2) Added new function parameter to communicate the timeout + status to callers. + (giop_timeout_add): New function to add a timeout thread. + (giop_timeout): Timeout thread function. Will wait using nanosleep + until the timeout expires. Will change the link state + to LINK_TIMEOUT and signal wait condition. + + * src/orb/orb-core/corba-object.c (ORBit_try_connection_T): + Handle a timed out link just like a disconnected link. + + * src/orb/orb-core/corba-orb.c: The ORB command line option: + + GIOPTimeoutLimit + + is no more. It has been changed into: + + GIOPTimeoutMSEC - IPv4/6 timeout limit in milliseconds + + It defaults to 60 seconds. This options controls the timeout + limit for GIOP connections, but only for IPv4 and IPv6 + connections. Setting it to zero will explicitly disable any + timeout control. + +2007-06-13 Jules Colding <colding@...> + + * src/orb/util/orbit-options.c (ORBit_option_set): Support parsing + unsigned long option values. + + * src/orb/util/orbit-options.h (enum): Added support for + unsigned long option values. + + * Reverted previous GIOP timeout patch after having discussed + my flawed approach with Michael. + 2007-03-18 Brian Cameron <Brian.Cameron@...> * linc2/src/linc-protocols.c: Now always set the _______________________________________________ orbit-list mailing list orbit-list@... http://mail.gnome.org/mailman/listinfo/orbit-list
	Re: New GIOP timeout patch - please review by colding :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message On Thu, 2007-06-21 at 13:09 +0100, Gustavo J. A. M. Carneiro wrote: > Hi, > > I must say that launching a thread for each outoging call just to get > a timeout is way overkill. BTW, it is not for each and every outgoing call. It is only for non-oneway IPV4 or IPv6 connections. Not for unix sockets and such. -- jules _______________________________________________ orbit-list mailing list orbit-list@... http://mail.gnome.org/mailman/listinfo/orbit-list
	Re: New GIOP timeout patch - please review by colding :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message Hi Michael, On Thu, 2007-06-21 at 12:46 +0100, Michael Meeks wrote: > Hi Jules, > > On Thu, 2007-06-21 at 14:49 +0200, Jules Colding wrote: > > As always... Just noticed two small and stupid bugs. Corrected patch > > below. > > Sigh - so 1 new thread per IPv4 connection ? that will cripple Sun who > use IPv4/CORBA to do their Java a11y bridging stuff. We should only do > that as a stop-gap, and only if it's turned off by default for IPv4 > users I think [ is that so ? ]. No, the default is 60 seconds. The reason is that it is currently impossible to change ORB options if you are not the lucky one to call ORB_init(). I could create a workaround, but that would add to the external API, so I assumed that a long default would be the better choice. > -really- it would be way better to instrument your glib and find out > why the timeout thing is not getting called. -One- possibility may be > that you're not setting it to be a 'recursive' source (although perhaps > unlikely given the lack of re-enterancy there). I think that I need to crawl down into glib and take a look... My current test patch is below as well as my modifications to the echo client/server. The echo server must be executed as: ./echo-server --ORBIIOPIPv4=1 --ORBLocalOnly=0 --ORBIIOPUNIX=0 The client should be executed as: ./echo-client --GIOPTimeoutMSEC=3 $(cat echo-server.iorfile) 1 I've attache timeout.h which I use to adjust the sleep time in the server. Any good ideas would be appreciated highly. Best regards, jules Index: src/orb/orb-core/corba-orb.c =================================================================== --- src/orb/orb-core/corba-orb.c (revision 2003) +++ src/orb/orb-core/corba-orb.c (working copy) @@ -61,7 +61,7 @@ static char orbit_naming_ref = NULL; static GSList orbit_initref_list = NULL; static gboolean orbit_use_corbaloc = FALSE; -static gint orbit_timeout_limit = -1; +static guint orbit_timeout_msec = 60000; /* 60 seconds - 0 will disable timeouts altogether / void ORBit_ORB_start_servers (CORBA_ORB orb) { @@ -418,7 +418,7 @@ #endif / G_ENABLE_DEBUG / giop_recv_set_limit (orbit_initial_recv_limit); - giop_recv_set_timeout (orbit_timeout_limit); + giop_set_timeout (orbit_timeout_msec); giop_init (thread_safe, orbit_use_ipv4 \|\| orbit_use_ipv6 \|\| orbit_use_irda \|\| orbit_use_ssl); @@ -1468,7 +1468,7 @@ { "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 }, + { "GIOPTimeoutMSEC", ORBIT_OPTION_ULONG, &orbit_timeout_msec }, { NULL, 0, NULL } }; Index: src/orb/orb-core/corba-object.c =================================================================== --- src/orb/orb-core/corba-object.c (revision 2003) +++ src/orb/orb-core/corba-object.c (working copy) @@ -270,6 +270,7 @@ retval = TRUE; break; case LINK_DISCONNECTED: + case LINK_TIMEOUT: / Have a go at reviving it / dprintf (MESSAGES, "re-connecting dropped cnx %p: ", cnx); if (giop_connection_try_reconnect (GIOP_CONNECTION (cnx)) == LINK_CONNECTED) Index: src/orb/util/orbit-options.c =================================================================== --- src/orb/util/orbit-options.c (revision 2003) +++ src/orb/util/orbit-options.c (working copy) @@ -53,6 +53,9 @@ case ORBIT_OPTION_INT: (gint )option->arg = atoi (val); break; + case ORBIT_OPTION_ULONG: + (guint )option->arg = strtoul(val, (char )NULL, 10); + break; case ORBIT_OPTION_STRING: { gchar str_arg = (char ) option->arg; Index: src/orb/util/orbit-options.h =================================================================== --- src/orb/util/orbit-options.h (revision 2003) +++ src/orb/util/orbit-options.h (working copy) @@ -10,7 +10,8 @@ ORBIT_OPTION_STRING, ORBIT_OPTION_INT, ORBIT_OPTION_BOOLEAN, - ORBIT_OPTION_KEY_VALUE / returns GSList of ORBit_option_key_value / + ORBIT_OPTION_KEY_VALUE, / returns GSList of ORBit_option_key_value / + ORBIT_OPTION_ULONG, } ORBit_option_type; typedef struct { Index: src/orb/GIOP/giop-recv-buffer.c =================================================================== --- src/orb/GIOP/giop-recv-buffer.c (revision 2003) +++ src/orb/GIOP/giop-recv-buffer.c (working copy) @@ -2,6 +2,8 @@ #include <string.h> #include <stdio.h> #include <errno.h> +#include <glib.h> +#include <glib/gprintf.h> #include "giop-private.h" #include "giop-debug.h" @@ -686,26 +688,20 @@ static inline gboolean check_got (GIOPMessageQueueEntry ent) { - return (ent->buffer \|\| !ent->cnx \|\| - (ent->cnx->parent.status == LINK_DISCONNECTED)); + return (ent->buffer \|\| + !ent->cnx \|\| + (ent->cnx->parent.status == LINK_DISCONNECTED) \|\| + (ent->cnx->parent.status == LINK_TIMEOUT)); } -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, gboolean timeout) { GIOPThread tdata = giop_thread_self (); - GTimeVal tval; + timeout = FALSE; + thread_switch: if (giop_thread_io ()) { ent_lock (ent); @@ -715,17 +711,8 @@ ent_unlock (ent); giop_thread_queue_process (tdata); ent_lock (ent); - } 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; - } + } else + g_cond_wait (tdata->incoming, tdata->lock); } ent_unlock (ent); @@ -734,6 +721,7 @@ while (!ent->buffer && ent->cnx && (ent->cnx->parent.status != LINK_DISCONNECTED) && + (ent->cnx->parent.status != LINK_TIMEOUT) && !giop_thread_io()) link_main_iteration (TRUE); @@ -741,6 +729,15 @@ goto thread_switch; } + if (ent->cnx->parent.timeout_mutex) { + g_mutex_lock (ent->cnx->parent.timeout_mutex); + if (ent->cnx->parent.timeout_status == LINK_TIMEOUT_UNKNOWN) + ent->cnx->parent.timeout_status = LINK_TIMEOUT_NO; + else + timeout = TRUE; + g_mutex_unlock (ent->cnx->parent.timeout_mutex); + } + giop_thread_queue_tail_wakeup (tdata); giop_recv_list_destroy_queue_entry (ent); @@ -1355,6 +1352,117 @@ return TRUE; } +struct timeout_thread_data { + GIOPThread tdata; + LinkConnection lcnx; +}; + +/ static gpointer / +/ giop_timeout(gpointer data) / +/ { / +/ LinkConnection lcnx = ((struct timeout_thread_data)data)->lcnx; / +/ GIOPThread tdata = ((struct timeout_thread_data)data)->tdata; / +/ struct timespec tv; / + +/ g_assert (lcnx->timeout_mutex); / + +/ tv.tv_sec = lcnx->timeout_msec / 1000; / +/ tv.tv_nsec = (lcnx->timeout_msec - tv.tv_sec1000) 1000000; / + +/ nanosleep (&tv, NULL); / + +/ g_mutex_lock (lcnx->timeout_mutex); / +/ if (lcnx->timeout_status == LINK_TIMEOUT_UNKNOWN) / +/ lcnx->timeout_status = LINK_TIMEOUT_YES; / +/ else { / +/ g_mutex_unlock (lcnx->timeout_mutex); / +/ goto out; / +/ } / +/ g_mutex_unlock (lcnx->timeout_mutex); / + +/ link_connection_state_changed (lcnx, LINK_TIMEOUT); / + +/ g_mutex_lock (tdata->lock); /\* ent_lock \/ / +/* giop_incoming_signal_T (tdata, GIOP_CLOSECONNECTION); / +/ g_mutex_unlock (tdata->lock); /\* ent_lock \/ / + +/* out: / +/ g_object_unref (lcnx); / +/ g_free (data); / + +/ return NULL; / +/ } / + +static gboolean +giop_timeout(gpointer data) +{ + LinkConnection lcnx = ((struct timeout_thread_data)data)->lcnx; + GIOPThread tdata = ((struct timeout_thread_data)data)->tdata; + + g_printf("Timeout function invoked\n"); + + g_assert (lcnx->timeout_mutex); + + g_mutex_lock (lcnx->timeout_mutex); + if (lcnx->timeout_status == LINK_TIMEOUT_UNKNOWN) + lcnx->timeout_status = LINK_TIMEOUT_YES; + else { + g_mutex_unlock (lcnx->timeout_mutex); + goto out; + } + g_mutex_unlock (lcnx->timeout_mutex); + + link_connection_state_changed (lcnx, LINK_TIMEOUT); + + g_mutex_lock (tdata->lock); / ent_lock / + giop_incoming_signal_T (tdata, GIOP_CLOSECONNECTION); + g_mutex_unlock (tdata->lock); / ent_lock / + +out: + g_object_unref (lcnx); + g_free (data); + + return FALSE; +} + +void +giop_timeout_add(GIOPConnection cnx) +{ + struct timeout_thread_data data = NULL; + LinkConnection lcnx = LINK_CONNECTION (cnx); + GSource timeout_source = NULL; + + if (!lcnx->timeout_msec) + return; + + g_object_ref (lcnx); + + if (!lcnx->timeout_mutex) + lcnx->timeout_mutex = g_mutex_new (); + + g_mutex_lock (lcnx->timeout_mutex); + lcnx->timeout_status = LINK_TIMEOUT_UNKNOWN; + g_mutex_unlock (lcnx->timeout_mutex); + + data = g_new0 (struct timeout_thread_data, 1); + data->tdata = giop_thread_self (); + data->lcnx = lcnx; + + g_printf("Adding timeout for %d milliseconds\n", lcnx->timeout_msec); + +/ g_thread_create (giop_timeout, / +/ (gpointer)data, / +/ FALSE, / +/ NULL); / + + timeout_source = g_timeout_source_new (lcnx->timeout_msec); + g_source_set_priority (timeout_source, G_PRIORITY_HIGH_IDLE); + g_source_set_callback (timeout_source, giop_timeout, (gpointer)data, NULL); + g_source_set_can_recurse (timeout_source, TRUE); + g_source_attach (timeout_source, data->tdata->wake_context); + g_source_unref (timeout_source); +} + GIOPRecvBuffer giop_recv_buffer_use_buf (GIOPConnection cnx) { @@ -1372,4 +1480,3 @@ return buf; } - Index: src/orb/GIOP/giop-connection.c =================================================================== --- src/orb/GIOP/giop-connection.c (revision 2003) +++ src/orb/GIOP/giop-connection.c (working copy) @@ -108,6 +108,7 @@ } } + static void giop_connection_class_init (GIOPConnectionClass klass) { @@ -192,3 +193,9 @@ return link_connection_try_reconnect (LINK_CONNECTION (cnx)); } +void +giop_set_timeout (guint msec) +{ + link_set_timeout (msec); +} + Index: src/orb/GIOP/giop-send-buffer.c =================================================================== --- src/orb/GIOP/giop-send-buffer.c (revision 2003) +++ src/orb/GIOP/giop-send-buffer.c (working copy) @@ -45,6 +45,25 @@ static const GIOP_AddressingDisposition giop_1_2_target_type = GIOP_KeyAddr; +static gboolean +giop_send_buffer_is_oneway(const GIOPSendBuffer buf) +{ + g_assert (buf); + + switch (buf->giop_version) { + case GIOP_1_0: + case GIOP_1_1: + return (buf->msg.u.request_1_0.response_expected ? FALSE : TRUE); + case GIOP_1_2: + return (buf->msg.u.request_1_2.response_flags ? FALSE : TRUE); + default: + break; + } + g_assert_not_reached(); + + return TRUE; +} + GIOPSendBuffer giop_send_buffer_use_request (GIOPVersion giop_version, CORBA_unsigned_long request_id, @@ -426,6 +445,7 @@ gboolean blocking) { int retval; + LinkConnection lcnx = LINK_CONNECTION (cnx); static LinkWriteOpts non_block = NULL; if (!non_block) @@ -434,11 +454,15 @@ /* FIXME: if a FRAGMENT, assert the 8 byte tail align, &&\|\| giop_send_buffer_align (buf, 8); / - retval = link_connection_writev ( - (LinkConnection ) cnx, buf->iovecs, - buf->num_used, - blocking ? NULL : non_block); + if (lcnx->timeout_msec && !giop_send_buffer_is_oneway (buf)) { + giop_timeout_add (cnx); + } + retval = link_connection_writev (lcnx, + buf->iovecs, + buf->num_used, + blocking ? NULL : non_block); + if (!blocking && retval == LINK_IO_QUEUED_DATA) retval = 0; Index: linc2/include/linc/linc-connection.h =================================================================== --- linc2/include/linc/linc-connection.h (revision 2003) +++ linc2/include/linc/linc-connection.h (working copy) @@ -39,7 +39,8 @@ #define LINK_IS_CONNECTION(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), LINK_TYPE_CONNECTION)) #define LINK_IS_CONNECTION_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), LINK_TYPE_CONNECTION)) -typedef enum { LINK_CONNECTING, LINK_CONNECTED, LINK_DISCONNECTED } LinkConnectionStatus; +typedef enum { LINK_CONNECTING, LINK_CONNECTED, LINK_DISCONNECTED, LINK_TIMEOUT } LinkConnectionStatus; +typedef enum { LINK_TIMEOUT_UNKNOWN, LINK_TIMEOUT_YES, LINK_TIMEOUT_NO } LinkTimeoutStatus; typedef struct _LinkWriteOpts LinkWriteOpts; typedef struct _LinkConnectionPrivate LinkConnectionPrivate; @@ -61,6 +62,10 @@ LinkConnectionPrivate priv; GSList idle_broken_callbacks; + + GMutex timeout_mutex; + guint timeout_msec; + LinkTimeoutStatus timeout_status;; } LinkConnection; typedef struct { @@ -153,6 +158,9 @@ void link_connections_close (void); +/ set the link timeout in miliseconds / +extern void link_set_timeout (guint msec); + G_END_DECLS #endif / _LINK_CONNECTION_H / Index: linc2/include/linc/linc.h =================================================================== --- linc2/include/linc/linc.h (revision 2003) +++ linc2/include/linc/linc.h (working copy) @@ -33,7 +33,7 @@ guint link_main_idle_add (GSourceFunc function, gpointer data); -gboolean link_wait (void); +void link_wait (void); void link_signal (void); gboolean link_thread_io (void); Index: linc2/ChangeLog =================================================================== --- linc2/ChangeLog (revision 2003) +++ linc2/ChangeLog (working copy) @@ -1,3 +1,24 @@ +2007-06-19 Jules Colding <colding@...> + + src/linc-connection.c (link_connection_init): Initialize + timeout members in the LinkConnection structure. + (link_set_timeout): New function to set the timeout value. + +2007-06-18 Jules Colding <colding@...> + + * src/linc-connection.c (link_connection_from_fd_T): Initiate + timeout members of the link connection iff: + 1) The connection is IPv4 or IPv6 + 2) It is not a oneway + 3) The timeout parameter is non-zero + + * include/linc/linc-connection.h (struct): + 1) timeout mutex + 2) timeout value in milliseconds + 3) timeout status + + Furthermore declare link_set_timeout() + ========================== ORBit2-2.14.8 ======================== 2007-02-27 Kjartan Maraas <kmaraas@...> Index: linc2/src/linc-debug.h =================================================================== --- linc2/src/linc-debug.h (revision 2003) +++ linc2/src/linc-debug.h (working copy) @@ -25,6 +25,7 @@ # define STATE_NAME(s) (((s) == LINK_CONNECTED) ? "Connected" : \ ((s) == LINK_CONNECTING) ? "Connecting" : \ ((s) == LINK_DISCONNECTED) ? "Disconnected" : \ + ((s) == LINK_TIMEOUT) ? "Timeout" : \ "Invalid state") # ifdef CONNECTION_DEBUG_FLAG extern gboolean link_connection_debug_flag; Index: linc2/src/linc-connection.c =================================================================== --- linc2/src/linc-connection.c (revision 2003) +++ linc2/src/linc-connection.c (working copy) @@ -27,6 +27,7 @@ #include <linc/linc-connection.h> static GObjectClass parent_class = NULL; +static guint _link_timeout = 0; enum { BROKEN, @@ -288,6 +289,7 @@ break; case LINK_DISCONNECTED: + case LINK_TIMEOUT: link_source_remove (cnx); link_close_fd (cnx); queue_free (cnx); @@ -440,6 +442,16 @@ g_free (cnx->remote_serv_info); cnx->remote_serv_info = remote_serv_info; + switch (cnx->proto->family) { + case AF_INET: + case AF_INET6: + if (_link_timeout && !cnx->timeout_msec) / this should'nt happen twice but I'm always paranoid... / + cnx->timeout_msec = _link_timeout; + break; + default: + break; + } + d_printf ("Cnx from fd (%d) '%s', '%s', '%s'\n", fd, proto->name, remote_host_info ? remote_host_info : "<Null>", @@ -635,10 +647,8 @@ static LinkConnectionStatus link_connection_wait_connected_T (LinkConnection cnx) { - while (cnx && cnx->status == LINK_CONNECTING) { - if (!link_wait ()) - link_connection_disconnect (cnx); - } + while (cnx && cnx->status == LINK_CONNECTING) + link_wait (); return cnx ? cnx->status : LINK_DISCONNECTED; } @@ -661,12 +671,8 @@ cnx->inhibit_reconnect = FALSE; dispatch_callbacks_drop_lock (cnx); g_main_context_release (NULL); - } else { - if (!link_wait ()) { - link_connection_disconnect (cnx); - break; - } - } + } else + link_wait (); } if (cnx->status != LINK_DISCONNECTED) @@ -1254,6 +1260,9 @@ g_free (cnx->priv); + if (cnx->timeout_mutex) + g_mutex_free (cnx->timeout_mutex); + #ifdef G_ENABLE_DEBUG g_assert (g_list_find(cnx_list, cnx) == NULL); #endif @@ -1269,6 +1278,11 @@ cnx->priv = g_new0 (LinkConnectionPrivate, 1); cnx->priv->fd = -1; cnx->priv->was_disconnected = FALSE; + + cnx->timeout_mutex = NULL; + cnx->timeout_msec = 0; + cnx->timeout_status = LINK_TIMEOUT_UNKNOWN; + #ifdef CONNECTION_DEBUG cnx->priv->total_read_bytes = 0; cnx->priv->total_written_bytes = 0; @@ -1568,3 +1582,10 @@ g_list_free (cnx); } + +void +link_set_timeout (guint msec) +{ + _link_timeout = msec; +} + Index: linc2/src/linc.c =================================================================== --- linc2/src/linc.c (revision 2003) +++ linc2/src/linc.c (working copy) @@ -52,9 +52,6 @@ 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 @@ -537,28 +534,17 @@ } } -gboolean +void 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_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; - } + g_cond_wait (link_main_cond, link_main_lock); } - - return TRUE; } Index: test/echo-server.c =================================================================== --- test/echo-server.c (revision 2003) +++ test/echo-server.c (working copy) @@ -40,7 +40,8 @@ signal(SIGTERM, exit); CORBA_exception_init(&ev); - orb = CORBA_ORB_init(&argc, argv, "orbit-local-orb", &ev); +// orb = CORBA_ORB_init(&argc, argv, "orbit-local-orb", &ev); + orb = CORBA_ORB_init(&argc, argv, "orbit-io-thread", &ev); g_assert(ev._major == CORBA_NO_EXCEPTION); echo_srv_start_poa(orb, &ev); Index: test/echo-srv.c =================================================================== --- test/echo-srv.c (revision 2003) +++ test/echo-srv.c (working copy) @@ -22,8 +22,10 @@ #include <stdlib.h> #include <string.h> #include <ctype.h> +#include <unistd.h> #include "echo.h" #include "echo-share.h" +#include "timeout.h" /* This is used by echo-server.c and echo-local.c @@ -47,7 +49,9 @@ g_message ("[server] %s", astring); outnum = rand() % 100; - + + sleep(SLEEP); + return CORBA_Object_duplicate (the_echo_client, ev); } Index: test/echo-client.c =================================================================== --- test/echo-client.c (revision 2003) +++ test/echo-client.c (working copy) @@ -21,15 +21,24 @@ #include <stdlib.h> #include "echo.h" +#include "timeout.h" #define ABORT_IF_EXCEPTION(_ev, _message) \ -if ((_ev)->_major != CORBA_NO_EXCEPTION) { \ - g_error("%s: %s", _message, CORBA_exception_id (_ev)); \ + if ((_ev) && (_ev)->_major != CORBA_NO_EXCEPTION) { \ + g_error("%s: %s\n", _message, CORBA_exception_id (_ev)); \ CORBA_exception_free (_ev); \ abort(); \ } +#define PRINT_EXCEPTION(_ev, _message) do { \ + if (_ev) \ + g_print("%s: %s\n", _message, CORBA_exception_id (_ev)); \ + else \ + g_print("No exception\n"); \ + } while (0); + + static Echo echo_client, bec; static gboolean echo_opt_quiet = FALSE; @@ -45,7 +54,8 @@ int niters = 1000; CORBA_exception_init(&ev); - orb = CORBA_ORB_init(&argc, argv, "orbit-local-orb", &ev); +// orb = CORBA_ORB_init(&argc, argv, "orbit-local-orb", &ev); + orb = CORBA_ORB_init(&argc, argv, "orbit-io-thread", &ev); / read IOR from command line as first argument / if(argc < 2) { @@ -73,32 +83,51 @@ for(i = 0; i < niters; i++) { / Method call without any argument, usefull to tell * lifeness / - Echo_doNothing(echo_client, &ev); - ABORT_IF_EXCEPTION (&ev, "service raised exception "); +/ Echo_doNothing(echo_client, &ev); / +/ ABORT_IF_EXCEPTION (&ev, "service raised exception "); / / Ask echo-service to print string 'buf' on console. The * service returns random double float value in 'vr' / - g_snprintf(buf, sizeof(buf), "Hello, world [%d]", i); +/ g_snprintf(buf, sizeof(buf), "Hello, world [%d]", i); / +/ bec = Echo_echoString(echo_client, buf, &rv, &ev); / +/ ABORT_IF_EXCEPTION (&ev, "service raised exception "); / + + / Test the GIOP timeout functionality. The server must be started + * with "--GIOPTimeoutLimit=2000 --ORBIIOPIPv4=1" for this test to work. / + g_snprintf(buf, sizeof(buf), TIMEOUT_STR); + g_message("0"); bec = Echo_echoString(echo_client, buf, &rv, &ev); - ABORT_IF_EXCEPTION (&ev, "service raised exception "); + g_message("00"); + if (ev._major != CORBA_NO_EXCEPTION) { + g_message("1"); + PRINT_EXCEPTION (&ev, "Printing exception "); + g_message("2"); + bec = CORBA_OBJECT_NIL; + g_message("3"); + } + CORBA_exception_init(&ev); + g_message("A"); / print random value generated by echo-service / - if ( !echo_opt_quiet ) - g_message("[client] %g", rv); +/ if ( !echo_opt_quiet ) / +/ g_message("[client] %g", rv); / / Asynchronous/oneway method call, the function returns * immediately. Usefull for log-message transfer / - Echo_doOneWay(echo_client, "log message ", &ev); - ABORT_IF_EXCEPTION (&ev, "service raised exception "); +/ Echo_doOneWay(echo_client, "log message ", &ev); / +/ ABORT_IF_EXCEPTION (&ev, "service raised exception "); / / release first object reference and use the new one for * next loop / CORBA_Object_release(echo_client, &ev); + g_message("B"); ABORT_IF_EXCEPTION (&ev, "service raised exception "); + g_message("C"); / swap object references / echo_client = bec; bec = CORBA_OBJECT_NIL; } + g_message("C"); / release initial object reference / CORBA_Object_release(echo_client, &ev); Index: include/orbit/GIOP/giop-connection.h =================================================================== --- include/orbit/GIOP/giop-connection.h (revision 2003) +++ include/orbit/GIOP/giop-connection.h (working copy) @@ -53,6 +53,9 @@ #define giop_connection_ref(cnx) link_connection_ref(cnx) #define giop_connection_unref(cnx) link_connection_unref(cnx) +/ set the link timeout in milliseconds / +extern void giop_set_timeout (guint msec); + #endif / ORBIT2_INTERNAL_API / G_END_DECLS Index: include/orbit/GIOP/giop.h =================================================================== --- include/orbit/GIOP/giop.h (revision 2003) +++ include/orbit/GIOP/giop.h (working copy) @@ -23,7 +23,6 @@ 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); @@ -47,7 +46,6 @@ gboolean giop_thread_queue_empty_T (GIOPThread tdata); void giop_thread_queue_tail_wakeup(GIOPThread tdata); - #endif /* ORBIT2_INTERNAL_API / G_END_DECLS Index: include/orbit/GIOP/giop-types.h =================================================================== --- include/orbit/GIOP/giop-types.h (revision 2003) +++ include/orbit/GIOP/giop-types.h (working copy) @@ -35,10 +35,8 @@ gpointer dummy); }; -#define GIOP_INITIAL_TIMEOUT_LIMIT (30000000) / 30 seconds / +#define GIOP_INITIAL_MSG_SIZE_LIMIT (2561024) -#define GIOP_INITIAL_MSG_SIZE_LIMIT (2561024) / in bytes / - typedef enum { GIOP_CONNECTION_SSL } GIOPConnectionOptions; Index: include/orbit/GIOP/giop-recv-buffer.h =================================================================== --- include/orbit/GIOP/giop-recv-buffer.h (revision 2003) +++ include/orbit/GIOP/giop-recv-buffer.h (working copy) @@ -74,7 +74,9 @@ void giop_recv_list_zap (GIOPConnection cnx); gboolean giop_connection_handle_input (LinkConnection lcnx); void giop_connection_destroy_frags (GIOPConnection cnx); +extern void giop_timeout_add (GIOPConnection cnx); + #endif / ORBIT2_INTERNAL_API / G_END_DECLS Index: ChangeLog =================================================================== --- ChangeLog (revision 2003) +++ ChangeLog (working copy) @@ -1,3 +1,49 @@ +2007-06-18 Jules Colding <colding@...> + + src/orb/GIOP/giop-send-buffer.c (giop_send_buffer_is_oneway): + New function to tell if the connection is a oneway. + (giop_send_buffer_write): Will add a timeout for the connection + iff the timeout value is non-zero and the connection isn't oneway. + + * src/orb/GIOP/giop-connection.c (giop_set_timeout): New function + to set the link timeout value. + + * src/orb/GIOP/giop-recv-buffer.c (giop_recv_buffer_get): + 1) Handle the LINK_TIMEOUT connection status. + 2) Added new function parameter to communicate the timeout + status to callers. + (giop_timeout_add): New function to add a timeout thread. + (giop_timeout): Timeout thread function. Will wait using nanosleep + until the timeout expires. Will change the link state + to LINK_TIMEOUT and signal wait condition. + + * src/orb/orb-core/corba-object.c (ORBit_try_connection_T): + Handle a timed out link just like a disconnected link. + + * src/orb/orb-core/corba-orb.c: The ORB command line option: + + GIOPTimeoutLimit + + is no more. It has been changed into: + + GIOPTimeoutMSEC - IPv4/6 timeout limit in milliseconds + + It defaults to 60 seconds. This options controls the timeout + limit for GIOP connections, but only for IPv4 and IPv6 + connections. Setting it to zero will explicitly disable any + timeout control. + +2007-06-13 Jules Colding <colding@...> + + * src/orb/util/orbit-options.c (ORBit_option_set): Support parsing + unsigned long option values. + + * src/orb/util/orbit-options.h (enum): Added support for + unsigned long option values. + + * Reverted previous GIOP timeout patch after having discussed + my flawed approach with Michael. + 2007-03-18 Brian Cameron <Brian.Cameron@...> * linc2/src/linc-protocols.c: Now always set the `[timeout.h]` /* * CORBA timeout test values * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU General Public License as published by the * Free Software Foundation; either version 2, or (at your option) any * later version. * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. * * You should have received a copy of the GNU General Public License * along with this program; if not, write to the Free Software Foundation, * Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. * * Author: Jules Colding <colding@.com> */ #ifndef timeout_H #define timeout_H (1) #define TIMEOUT (2) #define SLEEP (TIMEOUT+1) #define TIMEOUT_STR "test_timeout" #endif _______________________________________________ orbit-list mailing list orbit-list@... http://mail.gnome.org/mailman/listinfo/orbit-list
	Re: New GIOP timeout patch - please review by michael meeks :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message Hi Jules, On Fri, 2007-06-22 at 14:18 +0200, Jules Colding wrote: > I think that I need to crawl down into glib and take a look... My > current test patch is below as well as my modifications to the echo > client/server. The echo server must be executed as: ;-) we're attaching to the wrong main context: /echo-client --GIOPTimeoutMSEC=30 `cat /tmp/ior` 1 Message: 0 Adding timeout for 30 milliseconds Timeout function invoked Message: 00 Message: 1 Printing exception : IDL:omg.org/CORBA/TIMEOUT:1.0 Message: 2 Message: 3 Message: A Message: B Message: C ** Message: C I attach a patch that works for me [ I was missing timeout.h FWIW ]... Things to fix: * lifecycle, we can't just add timeouts without removing them again if our connection is closed. * adding a timeout is quite expensive CPU-wise; we might do better by simply extending linc-source.c (link_source_prepare) to handle the timeouts & manage how long each connection has been waiting itself: to avoid adding many more sources. [ though not sure how to get notified of that timeout - check the glib/gmain.c source I guess ]. HTH, Michael. -- michael.meeks@... <><, Pseudo Engineer, itinerant idiot `[for-jules.diff]` _______________________________________________ orbit-list mailing list orbit-list@... http://mail.gnome.org/mailman/listinfo/orbit-list
	Re: New GIOP timeout patch - please review by colding :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message Hi, On Fri, 2007-06-22 at 14:18 +0200, Jules Colding wrote: > > -really- it would be way better to instrument your glib and find out > > why the timeout thing is not getting called. -One- possibility may be > > that you're not setting it to be a 'recursive' source (although perhaps > > unlikely given the lack of re-enterancy there). > > I think that I need to crawl down into glib and take a look... Sigh... I've been crawling all over the glib source. I can see that my timeout source is being added to the context(s) but nothing happens. I've tried the main context and the giop loop context but with equally disappointing results. Any advise/help/encouraging yells would be highly appreciated. Best regards, jules _______________________________________________ orbit-list mailing list orbit-list@... http://mail.gnome.org/mailman/listinfo/orbit-list
	Re: New GIOP timeout patch - please review by Gustavo J. A. M. Carneiro :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message On Sex, 2007-06-22 at 16:06 +0200, Jules Colding wrote: > Hi, > > On Fri, 2007-06-22 at 14:18 +0200, Jules Colding wrote: > > > -really- it would be way better to instrument your glib and find out > > > why the timeout thing is not getting called. -One- possibility may be > > > that you're not setting it to be a 'recursive' source (although perhaps > > > unlikely given the lack of re-enterancy there). > > > > I think that I need to crawl down into glib and take a look... > > Sigh... I've been crawling all over the glib source. I can see that my > timeout source is being added to the context(s) but nothing happens. > > I've tried the main context and the giop loop context but with equally > disappointing results. > > Any advise/help/encouraging yells would be highly appreciated. Try using strace to run your test program. Make sure the the Link socket is always in non-blocking mode; if it is in blocking mode, for sure glib timeouts will not work. -- Gustavo J. A. M. Carneiro INESC Porto, Telecommunications and Multimedia Unit "The universe is always one step beyond logic." -- Frank Herbert _______________________________________________ orbit-list mailing list orbit-list@... http://mail.gnome.org/mailman/listinfo/orbit-list
	Re: New GIOP timeout patch - please review by colding :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message On Fri, 2007-06-22 at 13:31 +0100, Michael Meeks wrote: > Hi Jules, > > On Fri, 2007-06-22 at 14:18 +0200, Jules Colding wrote: > > I think that I need to crawl down into glib and take a look... My > > current test patch is below as well as my modifications to the echo > > client/server. The echo server must be executed as: > > ;-) we're attaching to the wrong main context: OK -that would explain everything ;-) Thanks a lot for spotting that. > I attach a patch that works for me [ I was missing timeout.h FWIW ]... > > Things to fix: > > * lifecycle, we can't just add timeouts without removing them > again if our connection is closed. This is fixed in my new patch below. > * adding a timeout is quite expensive CPU-wise; we might do > better by simply extending linc-source.c (link_source_prepare) > to handle the timeouts & manage how long each connection has > been waiting itself: to avoid adding many more sources. > [ though not sure how to get notified of that timeout - check > the glib/gmain.c source I guess ]. Hmm... I like to wait a bit with this one. I think that my current fumbling around has lasted long enough and that we need to get the correted timeout approach into mainline asap. The above can always be attended to later. The new patch is below. Please ignore the changes to the echo client/server. They will be reverted before the patch is committed. Comments? Best regards, jules Index: src/orb/orb-core/corba-orb.c =================================================================== --- src/orb/orb-core/corba-orb.c (revision 2003) +++ src/orb/orb-core/corba-orb.c (working copy) @@ -61,7 +61,7 @@ static char orbit_naming_ref = NULL; static GSList orbit_initref_list = NULL; static gboolean orbit_use_corbaloc = FALSE; -static gint orbit_timeout_limit = -1; +static guint orbit_timeout_msec = 60000; /* 60 seconds - 0 will disable timeouts altogether / void ORBit_ORB_start_servers (CORBA_ORB orb) { @@ -418,7 +418,7 @@ #endif / G_ENABLE_DEBUG / giop_recv_set_limit (orbit_initial_recv_limit); - giop_recv_set_timeout (orbit_timeout_limit); + giop_set_timeout (orbit_timeout_msec); giop_init (thread_safe, orbit_use_ipv4 \|\| orbit_use_ipv6 \|\| orbit_use_irda \|\| orbit_use_ssl); @@ -1468,7 +1468,7 @@ { "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 }, + { "GIOPTimeoutMSEC", ORBIT_OPTION_ULONG, &orbit_timeout_msec }, { NULL, 0, NULL } }; Index: src/orb/orb-core/corba-object.c =================================================================== --- src/orb/orb-core/corba-object.c (revision 2003) +++ src/orb/orb-core/corba-object.c (working copy) @@ -270,6 +270,7 @@ retval = TRUE; break; case LINK_DISCONNECTED: + case LINK_TIMEOUT: / Have a go at reviving it / dprintf (MESSAGES, "re-connecting dropped cnx %p: ", cnx); if (giop_connection_try_reconnect (GIOP_CONNECTION (cnx)) == LINK_CONNECTED) Index: src/orb/util/orbit-options.c =================================================================== --- src/orb/util/orbit-options.c (revision 2003) +++ src/orb/util/orbit-options.c (working copy) @@ -53,6 +53,9 @@ case ORBIT_OPTION_INT: (gint )option->arg = atoi (val); break; + case ORBIT_OPTION_ULONG: + (guint )option->arg = strtoul(val, (char )NULL, 10); + break; case ORBIT_OPTION_STRING: { gchar str_arg = (char ) option->arg; Index: src/orb/util/orbit-options.h =================================================================== --- src/orb/util/orbit-options.h (revision 2003) +++ src/orb/util/orbit-options.h (working copy) @@ -10,7 +10,8 @@ ORBIT_OPTION_STRING, ORBIT_OPTION_INT, ORBIT_OPTION_BOOLEAN, - ORBIT_OPTION_KEY_VALUE / returns GSList of ORBit_option_key_value / + ORBIT_OPTION_KEY_VALUE, / returns GSList of ORBit_option_key_value / + ORBIT_OPTION_ULONG, } ORBit_option_type; typedef struct { Index: src/orb/GIOP/giop-recv-buffer.c =================================================================== --- src/orb/GIOP/giop-recv-buffer.c (revision 2003) +++ src/orb/GIOP/giop-recv-buffer.c (working copy) @@ -2,6 +2,8 @@ #include <string.h> #include <stdio.h> #include <errno.h> +#include <glib.h> +#include <glib/gprintf.h> #include "giop-private.h" #include "giop-debug.h" @@ -686,26 +688,20 @@ static inline gboolean check_got (GIOPMessageQueueEntry ent) { - return (ent->buffer \|\| !ent->cnx \|\| - (ent->cnx->parent.status == LINK_DISCONNECTED)); + return (ent->buffer \|\| + !ent->cnx \|\| + (ent->cnx->parent.status == LINK_DISCONNECTED) \|\| + (ent->cnx->parent.status == LINK_TIMEOUT)); } -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, gboolean timeout) { GIOPThread tdata = giop_thread_self (); - GTimeVal tval; + timeout = FALSE; + thread_switch: if (giop_thread_io ()) { ent_lock (ent); @@ -715,17 +711,8 @@ ent_unlock (ent); giop_thread_queue_process (tdata); ent_lock (ent); - } 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; - } + } else + g_cond_wait (tdata->incoming, tdata->lock); } ent_unlock (ent); @@ -734,6 +721,7 @@ while (!ent->buffer && ent->cnx && (ent->cnx->parent.status != LINK_DISCONNECTED) && + (ent->cnx->parent.status != LINK_TIMEOUT) && !giop_thread_io()) link_main_iteration (TRUE); @@ -741,6 +729,17 @@ goto thread_switch; } + if (ent->cnx->parent.timeout_mutex) { + g_mutex_lock (ent->cnx->parent.timeout_mutex); + if (ent->cnx->parent.timeout_status == LINK_TIMEOUT_UNKNOWN) { + link_io_thread_remove_timeout (ent->cnx->parent.timeout_source_id); + ent->cnx->parent.timeout_source_id = 0; + ent->cnx->parent.timeout_status = LINK_TIMEOUT_NO; + } else + timeout = TRUE; + g_mutex_unlock (ent->cnx->parent.timeout_mutex); + } + giop_thread_queue_tail_wakeup (tdata); giop_recv_list_destroy_queue_entry (ent); @@ -1355,6 +1354,110 @@ return TRUE; } +struct timeout_thread_data { + GIOPThread tdata; + LinkConnection lcnx; +}; + +/ static gpointer / +/ giop_timeout(gpointer data) / +/ { / +/ LinkConnection lcnx = ((struct timeout_thread_data)data)->lcnx; / +/ GIOPThread tdata = ((struct timeout_thread_data)data)->tdata; / +/ struct timespec tv; / + +/ g_assert (lcnx->timeout_mutex); / + +/ tv.tv_sec = lcnx->timeout_msec / 1000; / +/ tv.tv_nsec = (lcnx->timeout_msec - tv.tv_sec1000) 1000000; / + +/ nanosleep (&tv, NULL); / + +/ g_mutex_lock (lcnx->timeout_mutex); / +/ if (lcnx->timeout_status == LINK_TIMEOUT_UNKNOWN) / +/ lcnx->timeout_status = LINK_TIMEOUT_YES; / +/ else { / +/ g_mutex_unlock (lcnx->timeout_mutex); / +/ goto out; / +/ } / +/ g_mutex_unlock (lcnx->timeout_mutex); / + +/ link_connection_state_changed (lcnx, LINK_TIMEOUT); / + +/ g_mutex_lock (tdata->lock); /\* ent_lock \/ / +/* giop_incoming_signal_T (tdata, GIOP_CLOSECONNECTION); / +/ g_mutex_unlock (tdata->lock); /\* ent_lock \/ / + +/* out: / +/ g_object_unref (lcnx); / +/ g_free (data); / + +/ return NULL; / +/ } / + +static gboolean +giop_timeout(gpointer data) +{ + gboolean retv = FALSE; + LinkConnection lcnx = ((struct timeout_thread_data)data)->lcnx; + GIOPThread tdata = ((struct timeout_thread_data)data)->tdata; + + g_printf("Timeout function invoked\n"); + + g_assert (lcnx->timeout_mutex); + + g_mutex_lock (lcnx->timeout_mutex); + if (lcnx->timeout_status == LINK_TIMEOUT_UNKNOWN) { + lcnx->timeout_source_id = 0; + lcnx->timeout_status = LINK_TIMEOUT_YES; + } else { + g_mutex_unlock (lcnx->timeout_mutex); + retv = TRUE; // do not remove the source - the one who sets timeout_status will do that + goto out; + } + g_mutex_unlock (lcnx->timeout_mutex); + + link_connection_state_changed (lcnx, LINK_TIMEOUT); + + g_mutex_lock (tdata->lock); / ent_lock / + giop_incoming_signal_T (tdata, GIOP_CLOSECONNECTION); + g_mutex_unlock (tdata->lock); / ent_lock / + +out: + g_object_unref (lcnx); + g_free (data); + + return retv; +} + +void +giop_timeout_add(GIOPConnection cnx) +{ + struct timeout_thread_data data = NULL; + LinkConnection lcnx = LINK_CONNECTION (cnx); + GSource timeout_source = NULL; + + if (!lcnx->timeout_msec) + return; + + g_object_ref (lcnx); + + if (!lcnx->timeout_mutex) + lcnx->timeout_mutex = g_mutex_new (); + + g_mutex_lock (lcnx->timeout_mutex); + lcnx->timeout_status = LINK_TIMEOUT_UNKNOWN; + g_mutex_unlock (lcnx->timeout_mutex); + + data = g_new0 (struct timeout_thread_data, 1); + data->tdata = giop_thread_self (); + data->lcnx = lcnx; + + g_printf("Adding timeout for %d milliseconds\n", lcnx->timeout_msec); + + lcnx->timeout_source_id = link_io_thread_add_timeout (lcnx->timeout_msec, giop_timeout, data); +} + GIOPRecvBuffer giop_recv_buffer_use_buf (GIOPConnection cnx) { @@ -1372,4 +1475,3 @@ return buf; } - Index: src/orb/GIOP/giop-connection.c =================================================================== --- src/orb/GIOP/giop-connection.c (revision 2003) +++ src/orb/GIOP/giop-connection.c (working copy) @@ -108,6 +108,7 @@ } } + static void giop_connection_class_init (GIOPConnectionClass klass) { @@ -192,3 +193,9 @@ return link_connection_try_reconnect (LINK_CONNECTION (cnx)); } +void +giop_set_timeout (guint msec) +{ + link_set_timeout (msec); +} + Index: src/orb/GIOP/giop-send-buffer.c =================================================================== --- src/orb/GIOP/giop-send-buffer.c (revision 2003) +++ src/orb/GIOP/giop-send-buffer.c (working copy) @@ -45,6 +45,25 @@ static const GIOP_AddressingDisposition giop_1_2_target_type = GIOP_KeyAddr; +static gboolean +giop_send_buffer_is_oneway(const GIOPSendBuffer buf) +{ + g_assert (buf); + + switch (buf->giop_version) { + case GIOP_1_0: + case GIOP_1_1: + return (buf->msg.u.request_1_0.response_expected ? FALSE : TRUE); + case GIOP_1_2: + return (buf->msg.u.request_1_2.response_flags ? FALSE : TRUE); + default: + break; + } + g_assert_not_reached(); + + return TRUE; +} + GIOPSendBuffer giop_send_buffer_use_request (GIOPVersion giop_version, CORBA_unsigned_long request_id, @@ -426,6 +445,7 @@ gboolean blocking) { int retval; + LinkConnection lcnx = LINK_CONNECTION (cnx); static LinkWriteOpts non_block = NULL; if (!non_block) @@ -434,11 +454,15 @@ /* FIXME: if a FRAGMENT, assert the 8 byte tail align, &&\|\| giop_send_buffer_align (buf, 8); / - retval = link_connection_writev ( - (LinkConnection ) cnx, buf->iovecs, - buf->num_used, - blocking ? NULL : non_block); + if (lcnx->timeout_msec && !giop_send_buffer_is_oneway (buf)) { + giop_timeout_add (cnx); + } + retval = link_connection_writev (lcnx, + buf->iovecs, + buf->num_used, + blocking ? NULL : non_block); + if (!blocking && retval == LINK_IO_QUEUED_DATA) retval = 0; Index: linc2/include/linc/linc-connection.h =================================================================== --- linc2/include/linc/linc-connection.h (revision 2003) +++ linc2/include/linc/linc-connection.h (working copy) @@ -39,7 +39,8 @@ #define LINK_IS_CONNECTION(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), LINK_TYPE_CONNECTION)) #define LINK_IS_CONNECTION_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), LINK_TYPE_CONNECTION)) -typedef enum { LINK_CONNECTING, LINK_CONNECTED, LINK_DISCONNECTED } LinkConnectionStatus; +typedef enum { LINK_CONNECTING, LINK_CONNECTED, LINK_DISCONNECTED, LINK_TIMEOUT } LinkConnectionStatus; +typedef enum { LINK_TIMEOUT_UNKNOWN, LINK_TIMEOUT_YES, LINK_TIMEOUT_NO } LinkTimeoutStatus; typedef struct _LinkWriteOpts LinkWriteOpts; typedef struct _LinkConnectionPrivate LinkConnectionPrivate; @@ -61,6 +62,11 @@ LinkConnectionPrivate priv; GSList idle_broken_callbacks; + + GMutex timeout_mutex; + guint timeout_msec; + guint timeout_source_id; // protected by timeout_mutex + LinkTimeoutStatus timeout_status; // protected by timeout_mutex } LinkConnection; typedef struct { @@ -153,6 +159,9 @@ void link_connections_close (void); +/ set the link timeout in miliseconds / +extern void link_set_timeout (guint msec); + G_END_DECLS #endif / _LINK_CONNECTION_H / Index: linc2/include/linc/linc.h =================================================================== --- linc2/include/linc/linc.h (revision 2003) +++ linc2/include/linc/linc.h (working copy) @@ -33,12 +33,17 @@ guint link_main_idle_add (GSourceFunc function, gpointer data); -gboolean link_wait (void); +void link_wait (void); void link_signal (void); gboolean link_thread_io (void); gboolean link_thread_safe (void); +guint link_io_thread_add_timeout (guint interval, + GSourceFunc function, + gpointer data); +void link_io_thread_remove_timeout (guint source_id); + #ifdef G_OS_WIN32 void link_map_winsock_error_to_errno (void); #endif Index: linc2/src/linc-debug.h =================================================================== --- linc2/src/linc-debug.h (revision 2003) +++ linc2/src/linc-debug.h (working copy) @@ -25,6 +25,7 @@ # define STATE_NAME(s) (((s) == LINK_CONNECTED) ? "Connected" : \ ((s) == LINK_CONNECTING) ? "Connecting" : \ ((s) == LINK_DISCONNECTED) ? "Disconnected" : \ + ((s) == LINK_TIMEOUT) ? "Timeout" : \ "Invalid state") # ifdef CONNECTION_DEBUG_FLAG extern gboolean link_connection_debug_flag; Index: linc2/src/linc-connection.c =================================================================== --- linc2/src/linc-connection.c (revision 2003) +++ linc2/src/linc-connection.c (working copy) @@ -27,6 +27,7 @@ #include <linc/linc-connection.h> static GObjectClass parent_class = NULL; +static guint _link_timeout = 0; enum { BROKEN, @@ -288,6 +289,7 @@ break; case LINK_DISCONNECTED: + case LINK_TIMEOUT: link_source_remove (cnx); link_close_fd (cnx); queue_free (cnx); @@ -440,6 +442,16 @@ g_free (cnx->remote_serv_info); cnx->remote_serv_info = remote_serv_info; + switch (cnx->proto->family) { + case AF_INET: + case AF_INET6: + if (_link_timeout && !cnx->timeout_msec) /* this should'nt happen twice but I'm always paranoid... / + cnx->timeout_msec = _link_timeout; + break; + default: + break; + } + d_printf ("Cnx from fd (%d) '%s', '%s', '%s'\n", fd, proto->name, remote_host_info ? remote_host_info : "<Null>", @@ -635,10 +647,8 @@ static LinkConnectionStatus link_connection_wait_connected_T (LinkConnection cnx) { - while (cnx && cnx->status == LINK_CONNECTING) { - if (!link_wait ()) - link_connection_disconnect (cnx); - } + while (cnx && cnx->status == LINK_CONNECTING) + link_wait (); return cnx ? cnx->status : LINK_DISCONNECTED; } @@ -661,12 +671,8 @@ cnx->inhibit_reconnect = FALSE; dispatch_callbacks_drop_lock (cnx); g_main_context_release (NULL); - } else { - if (!link_wait ()) { - link_connection_disconnect (cnx); - break; - } - } + } else + link_wait (); } if (cnx->status != LINK_DISCONNECTED) @@ -1254,6 +1260,13 @@ g_free (cnx->priv); + if (cnx->timeout_mutex) + g_mutex_free (cnx->timeout_mutex); + + if (cnx->timeout_source_id) + link_io_thread_remove_timeout (cnx->timeout_source_id); + + #ifdef G_ENABLE_DEBUG g_assert (g_list_find(cnx_list, cnx) == NULL); #endif @@ -1269,6 +1282,12 @@ cnx->priv = g_new0 (LinkConnectionPrivate, 1); cnx->priv->fd = -1; cnx->priv->was_disconnected = FALSE; + + cnx->timeout_mutex = NULL; + cnx->timeout_msec = 0; + cnx->timeout_source_id = 0; + cnx->timeout_status = LINK_TIMEOUT_UNKNOWN; + #ifdef CONNECTION_DEBUG cnx->priv->total_read_bytes = 0; cnx->priv->total_written_bytes = 0; @@ -1568,3 +1587,10 @@ g_list_free (cnx); } + +void +link_set_timeout (guint msec) +{ + _link_timeout = msec; +} + Index: linc2/src/linc.c =================================================================== --- linc2/src/linc.c (revision 2003) +++ linc2/src/linc.c (working copy) @@ -52,9 +52,6 @@ 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 @@ -537,28 +534,17 @@ } } -gboolean +void 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_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; - } + g_cond_wait (link_main_cond, link_main_lock); } - - return TRUE; } @@ -568,3 +554,34 @@ { return link_mutex_is_locked (link_main_lock); } + +/ Hack / +guint +link_io_thread_add_timeout (guint interval, + GSourceFunc function, + gpointer data) +{ + guint id; + GSource tsrc; + + tsrc = g_timeout_source_new (interval); + g_source_set_priority (tsrc, G_PRIORITY_HIGH_IDLE); + g_source_set_callback (tsrc, function, data, NULL); + g_source_set_can_recurse (tsrc, TRUE); + id = g_source_attach (tsrc, link_thread_context); + g_source_unref (tsrc); + + return id; +} + +void +link_io_thread_remove_timeout (guint source_id) +{ + GSource tsrc; + + if (!source_id) + return; + + tsrc = g_main_context_find_source_by_id (link_thread_context, source_id); + g_source_destroy (tsrc); +} Index: linc2/ChangeLog =================================================================== --- linc2/ChangeLog (revision 2003) +++ linc2/ChangeLog (working copy) @@ -1,3 +1,24 @@ +2007-06-19 Jules Colding <colding@...> + + src/linc-connection.c (link_connection_init): Initialize + timeout members in the LinkConnection structure. + (link_set_timeout): New function to set the timeout value. + +2007-06-18 Jules Colding <colding@...> + + * src/linc-connection.c (link_connection_from_fd_T): Initiate + timeout members of the link connection iff: + 1) The connection is IPv4 or IPv6 + 2) It is not a oneway + 3) The timeout parameter is non-zero + + * include/linc/linc-connection.h (struct): + 1) timeout mutex + 2) timeout value in milliseconds + 3) timeout status + + Furthermore declare link_set_timeout() + ========================== ORBit2-2.14.8 ======================== 2007-02-27 Kjartan Maraas <kmaraas@...> Index: test/echo-server.c =================================================================== --- test/echo-server.c (revision 2003) +++ test/echo-server.c (working copy) @@ -40,7 +40,8 @@ signal(SIGTERM, exit); CORBA_exception_init(&ev); - orb = CORBA_ORB_init(&argc, argv, "orbit-local-orb", &ev); +// orb = CORBA_ORB_init(&argc, argv, "orbit-local-orb", &ev); + orb = CORBA_ORB_init(&argc, argv, "orbit-io-thread", &ev); g_assert(ev._major == CORBA_NO_EXCEPTION); echo_srv_start_poa(orb, &ev); Index: test/echo-srv.c =================================================================== --- test/echo-srv.c (revision 2003) +++ test/echo-srv.c (working copy) @@ -22,8 +22,10 @@ #include <stdlib.h> #include <string.h> #include <ctype.h> +#include <unistd.h> #include "echo.h" #include "echo-share.h" +#include "timeout.h" /** This is used by echo-server.c and echo-local.c @@ -47,7 +49,9 @@ g_message ("[server] %s", astring); outnum = rand() % 100; - + + sleep(SLEEP); + return CORBA_Object_duplicate (the_echo_client, ev); } Index: test/echo-client.c =================================================================== --- test/echo-client.c (revision 2003) +++ test/echo-client.c (working copy) @@ -21,15 +21,24 @@ #include <stdlib.h> #include "echo.h" +#include "timeout.h" #define ABORT_IF_EXCEPTION(_ev, _message) \ -if ((_ev)->_major != CORBA_NO_EXCEPTION) { \ - g_error("%s: %s", _message, CORBA_exception_id (_ev)); \ + if ((_ev) && (_ev)->_major != CORBA_NO_EXCEPTION) { \ + g_error("%s: %s\n", _message, CORBA_exception_id (_ev)); \ CORBA_exception_free (_ev); \ abort(); \ } +#define PRINT_EXCEPTION(_ev, _message) do { \ + if (_ev) \ + g_print("%s: %s\n", _message, CORBA_exception_id (_ev)); \ + else \ + g_print("No exception\n"); \ + } while (0); + + static Echo echo_client, bec; static gboolean echo_opt_quiet = FALSE; @@ -45,7 +54,8 @@ int niters = 1000; CORBA_exception_init(&ev); - orb = CORBA_ORB_init(&argc, argv, "orbit-local-orb", &ev); +// orb = CORBA_ORB_init(&argc, argv, "orbit-local-orb", &ev); + orb = CORBA_ORB_init(&argc, argv, "orbit-io-thread", &ev); / read IOR from command line as first argument / if(argc < 2) { @@ -73,32 +83,51 @@ for(i = 0; i < niters; i++) { / Method call without any argument, usefull to tell * lifeness / - Echo_doNothing(echo_client, &ev); - ABORT_IF_EXCEPTION (&ev, "service raised exception "); +/ Echo_doNothing(echo_client, &ev); / +/ ABORT_IF_EXCEPTION (&ev, "service raised exception "); / / Ask echo-service to print string 'buf' on console. The * service returns random double float value in 'vr' / - g_snprintf(buf, sizeof(buf), "Hello, world [%d]", i); +/ g_snprintf(buf, sizeof(buf), "Hello, world [%d]", i); / +/ bec = Echo_echoString(echo_client, buf, &rv, &ev); / +/ ABORT_IF_EXCEPTION (&ev, "service raised exception "); / + + / Test the GIOP timeout functionality. The server must be started + * with "--GIOPTimeoutLimit=2000 --ORBIIOPIPv4=1" for this test to work. / + g_snprintf(buf, sizeof(buf), TIMEOUT_STR); + g_message("0"); bec = Echo_echoString(echo_client, buf, &rv, &ev); - ABORT_IF_EXCEPTION (&ev, "service raised exception "); + g_message("00"); + if (ev._major != CORBA_NO_EXCEPTION) { + g_message("1"); + PRINT_EXCEPTION (&ev, "Printing exception "); + g_message("2"); + bec = CORBA_OBJECT_NIL; + g_message("3"); + } + CORBA_exception_init(&ev); + g_message("A"); / print random value generated by echo-service / - if ( !echo_opt_quiet ) - g_message("[client] %g", rv); +/ if ( !echo_opt_quiet ) / +/ g_message("[client] %g", rv); / / Asynchronous/oneway method call, the function returns * immediately. Usefull for log-message transfer / - Echo_doOneWay(echo_client, "log message ", &ev); - ABORT_IF_EXCEPTION (&ev, "service raised exception "); +/ Echo_doOneWay(echo_client, "log message ", &ev); / +/ ABORT_IF_EXCEPTION (&ev, "service raised exception "); / / release first object reference and use the new one for * next loop / CORBA_Object_release(echo_client, &ev); + g_message("B"); ABORT_IF_EXCEPTION (&ev, "service raised exception "); + g_message("C"); / swap object references / echo_client = bec; bec = CORBA_OBJECT_NIL; } + g_message("C"); / release initial object reference / CORBA_Object_release(echo_client, &ev); Index: include/orbit/GIOP/giop-connection.h =================================================================== --- include/orbit/GIOP/giop-connection.h (revision 2003) +++ include/orbit/GIOP/giop-connection.h (working copy) @@ -53,6 +53,9 @@ #define giop_connection_ref(cnx) link_connection_ref(cnx) #define giop_connection_unref(cnx) link_connection_unref(cnx) +/ set the link timeout in milliseconds / +extern void giop_set_timeout (guint msec); + #endif / ORBIT2_INTERNAL_API / G_END_DECLS Index: include/orbit/GIOP/giop.h =================================================================== --- include/orbit/GIOP/giop.h (revision 2003) +++ include/orbit/GIOP/giop.h (working copy) @@ -23,7 +23,6 @@ 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); @@ -47,7 +46,6 @@ gboolean giop_thread_queue_empty_T (GIOPThread tdata); void giop_thread_queue_tail_wakeup(GIOPThread tdata); - #endif /* ORBIT2_INTERNAL_API / G_END_DECLS Index: include/orbit/GIOP/giop-types.h =================================================================== --- include/orbit/GIOP/giop-types.h (revision 2003) +++ include/orbit/GIOP/giop-types.h (working copy) @@ -35,10 +35,8 @@ gpointer dummy); }; -#define GIOP_INITIAL_TIMEOUT_LIMIT (30000000) / 30 seconds / +#define GIOP_INITIAL_MSG_SIZE_LIMIT (2561024) -#define GIOP_INITIAL_MSG_SIZE_LIMIT (2561024) / in bytes / - typedef enum { GIOP_CONNECTION_SSL } GIOPConnectionOptions; Index: include/orbit/GIOP/giop-recv-buffer.h =================================================================== --- include/orbit/GIOP/giop-recv-buffer.h (revision 2003) +++ include/orbit/GIOP/giop-recv-buffer.h (working copy) @@ -74,7 +74,9 @@ void giop_recv_list_zap (GIOPConnection cnx); gboolean giop_connection_handle_input (LinkConnection lcnx); void giop_connection_destroy_frags (GIOPConnection cnx); +extern void giop_timeout_add (GIOPConnection cnx); + #endif / ORBIT2_INTERNAL_API */ G_END_DECLS _______________________________________________ orbit-list mailing list orbit-list@... http://mail.gnome.org/mailman/listinfo/orbit-list

New GIOP timeout patch - please review

Re: New GIOP timeout patch - please review

New GIOP timeout patch - please review

Re: New GIOP timeout patch - please review

Re: New GIOP timeout patch - please review

Re: New GIOP timeout patch - please review

Re: New GIOP timeout patch - please review

Re: New GIOP timeout patch - please review

Re: New GIOP timeout patch - please review

Re: New GIOP timeout patch - please review

Re: New GIOP timeout patch - please review

Re: New GIOP timeout patch - please review