[omniORB-dev] Issue with omniORB select loop implementation

[Jonathan Biggar]

Duncan Grisby wrote:
>>Can anyone give me insight into the need for the 50 millisecond timer in 
>>the first place and what might happen if I changed it or removed it 
>>entirely?
> 
> 
> The time is used for two things, only one of which is actually
> necessary on Linux. When in the thread per connection mode (the
> default), just before the thread dedicated to a connection makes an
> upcall, it marks the thread as "selectable" by the thread doing
> select. The next time the thread wakes up, it starts watching the
> connection, in case the client sends interleaved calls on a single
> connection. That way, in the common case that a client serialises
> calls on each connection, there is no thread cross-talk, but the
> system still copes with clients that interleave calls.
> 
> The other thing it's used for is that on some platforms (notably
> Windows), it's not feasible to interrupt the thread doing select, so
> there's no way to add new connections to be watched. Having the fairly
> short timeout means that that's not too bad. On Unix platforms, a pipe
> is written to to wake up the select.
> 
> There's no particular reason for the timeout being hardcoded. It's
> just that nobody has ever seen the need to change it before. In most
> cases, on Linux, you could safely set it much larger. If you produce a
> patch to add a new configuration parameter for it, I'll include it in
> the release.

Thanks for the info.  The big problem we are having is that the 
application isn't really idle when it should be--no outstanding GIOP 
connections and still the 50ms timer is running, because the same 
Select() logic is used for the accept loop while waiting for new 
connections.

Here's a patch that I applied locally to our copy of 4.0.1 (we haven't 
upgraded yet) that does the trick for the accept problem.  It adds a 
flag to Select() that tells it not to run the timer.  This flag is only 
set when running the select loop in tcpEndpoint.cc, sslEndpoint.cc and 
unixEndpoint.cc.  Since all of those also have code that sets up a 
special "fake" connection to break out of the loop, this patch appears 
to cause no problems.

-- 
Jon Biggar
Levanta
jon at levanta.com
-------------- next part --------------
--- omniORB-4.0.1/include/omniORB4/internal/SocketCollection.h	2003-02-16 17:45:50.000000000 -0800
+++ ./include/omniORB4/internal/SocketCollection.h	2004-09-07 14:33:15.000000000 -0700
@@ -304,7 +304,7 @@
   void clearSelectable(SocketHandle_t);
   // Indicates that this connection need not be watched any more.
 
-  CORBA::Boolean Select();
+  CORBA::Boolean Select(CORBA::Boolean no_timer = 0);
   // Returns TRUE(1) if the Select() has successfully done a scan.
   // otherwise returns false(0) to indicate that an error has been
   // detected and this function should not be called again.
--- omniORB-4.0.1/src/lib/omniORB/orbcore/SocketCollection.cc	2003-02-17 02:39:52.000000000 -0800
+++ ./src/lib/omniORB/orbcore/SocketCollection.cc	2004-09-07 14:36:49.000000000 -0700
@@ -309,7 +309,7 @@
 
 /////////////////////////////////////////////////////////////////////////
 CORBA::Boolean
-SocketCollection::Select() {
+SocketCollection::Select(CORBA::Boolean no_timer) {
 
   struct timeval timeout;
   fd_set         rfds;
@@ -351,11 +351,15 @@
 
   int nready;
 
+  struct timeval *ptimeout = 0;
+
+  if (!no_timer)
+	  ptimeout = &timeout;
   if (fd != 0) {
 #ifndef GDB_DEBUG
-    nready = select(maxfd+1,&rfds,0,0,&timeout);
+    nready = select(maxfd+1,&rfds,0,0,ptimeout);
 #else
-    nready = do_select(maxfd+1,&rfds,0,0,&timeout);
+    nready = do_select(maxfd+1,&rfds,0,0,ptimeout);
 #endif
   }
   else {
--- omniORB-4.0.1/src/lib/omniORB/orbcore/ssl/sslEndpoint.cc	2003-02-16 18:03:10.000000000 -0800
+++ ./src/lib/omniORB/orbcore/ssl/sslEndpoint.cc	2004-09-07 14:30:30.000000000 -0700
@@ -352,7 +352,7 @@
 
   while (pd_go) {
     pd_new_conn_socket = RC_INVALID_SOCKET;
-    if (!Select()) break;
+    if (!Select(1)) break;
     if (pd_new_conn_socket != RC_INVALID_SOCKET) {
 
       ::SSL* ssl = SSL_new(pd_ctx->get_SSL_CTX());
--- omniORB-4.0.1/src/lib/omniORB/orbcore/tcp/tcpEndpoint.cc	2003-02-16 18:03:10.000000000 -0800
+++ ./src/lib/omniORB/orbcore/tcp/tcpEndpoint.cc	2004-09-07 14:30:45.000000000 -0700
@@ -342,7 +342,7 @@
 
   while (1) {
     pd_new_conn_socket = RC_INVALID_SOCKET;
-    if (!Select()) break;
+    if (!Select(1)) break;
     if (pd_new_conn_socket != RC_INVALID_SOCKET) {
       return  new tcpConnection(pd_new_conn_socket,this);
     }
--- omniORB-4.0.1/src/lib/omniORB/orbcore/unix/unixEndpoint.cc	2002-04-16 05:44:27.000000000 -0700
+++ ./src/lib/omniORB/orbcore/unix/unixEndpoint.cc	2004-09-07 15:46:08.000000000 -0700
@@ -206,7 +206,7 @@
 
   while (1) {
     pd_new_conn_socket = RC_INVALID_SOCKET;
-    if (!Select()) break;
+    if (!Select(1)) break;
     if (pd_new_conn_socket != RC_INVALID_SOCKET) {
       return  new unixConnection(pd_new_conn_socket,this,pd_filename,0);
     }