db-derby-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Apache Wiki <wikidi...@apache.org>
Subject [Db-derby Wiki] Update of "NetworkServerSessionManagement" by BryanPendleton
Date Sun, 21 May 2006 17:29:33 GMT
Dear Wiki user,

You have subscribed to a wiki page or wiki category on "Db-derby Wiki" for change notification.

The following page has been changed by BryanPendleton:
http://wiki.apache.org/db-derby/NetworkServerSessionManagement

The comment on the change is:
Add notes on synchronization

------------------------------------------------------------------------------
     Network Server restart leaves the Client Thread alone. This means that
     new {{{Session}}} may be coming in during the restart processing.
  
+ == Synchronization ==
+ 
+ There are several monitors used by the Network Server session management
+ code for synchronization:
+  * {{{sessionTable}}}
+  * {{{threadList}}}
+  * {{{runQueue}}}
+  * {{{serverStartSync}}}
+ 
+ === serverStartSync synchronization ===
+ 
+ The {{{serverStartSync}}} object has exactly one use, which is to single-thread
+ the flow through the {{{startNetworkServer()}}} method so that the network
+ server is always started (or restarted) exactly once, no matter how many
+ threads call this routine at the same time.
+ 
+ The {{{serverStartSync}}} monitor is used in conjunction with the
+ {{{restartFlag}}} boolean, and there is a comment at the head of the
+ {{{startNetworkServer()}}} method regarding the use of these two variables
+ which I don't understand.
+ 
+ === sessionTable synchronization ===
+ 
+ The {{{sessionTable}}} object is a Java Hashtable, which is an
+ automatically-synchronized collection object. Therefore calls to
+ {{{sessionTable.put()}}}, {{{sessionTable.get()}}} and
+ {{{sessionTable.remove()}}} do not need to be synchronized by the caller;
+ the Hashtable will perform internal synchronization automatically.
+ 
+ However, when traversing an enumeration of the elements in the Hashable,
+ the automatic synchronization does not help, so in these cases the Network
+ Server code explicitly encloses these traversals in {{{synchronized}}} blocks.
+ 
+ === threadList synchronization ===
+ 
+ The {{{threadList}}} object is a Java Vector, which is also an
+ automatically-synchronized collection object. So, again, calls to
+ {{{threadList.addElement()}}}, {{{threadList.get()}}},
+ {{{threadList.size()}}}, etc. do not need to be synchronized by the caller.
+ 
+ However, when traversing the entire threadList, the automatic synchronization
+ does not help, so in these cases the Network Server code explicitly
+ encloses the traversals in {{{synchronized}}} blocks.
+ 
+ Unfortunately, the Network Server code is inconsistent about these
+ traversals. These traversals are enclosed in {{{synchronized}}} blocks:
+  * in {{{blockingStart()}}} 
+  * in {{{setLogConnections}}}
+ However, these traversals are '''not''' enclosed in {{{synchronized}}} blocks:
+  * in {{{startNetworkServer()}}}
+  * in {{{buildRuntimeInfo()}}}
+  * in the {{{ClientThread}}} logic which decides whether to add a thread
+ 
+ This inconsistent synchronization is not explained in the code.
+ 
+ === runQueue synchronization ===
+ 
+ The {{{runQueue}}} object is also a Java Vector, so again it is an
+ automatically-synchronized object, and again the collection traversals need
+ to be explicitly synchronized.
+ 
+ Furthermore, the {{{runQueue}}} object is also used for {{{wait()}}} and
+ {{{notify()}}} calls, so some additional synchronization is needed to
+ perform these operations.
+ 
+ And the {{{runQueue}}} synchronization is also used to protect the
+ {{{freeThreads}}} variable, which is the count of the number of threads
+ currently waiting on the {{{runQueue}}} object.
+ 
+ Unfortunately, the Network Server code is again inconsistent in its use
+ of the explicit synchronization. There is inconsistent or missing
+ synchronization in at least these locations:
+  * in {{{startNetworkServer()}}}
+  * in {{{buildRuntimeInfo()}}}
+  * in the {{{ClientThread}}} logic which decides whether to add a thread
+ And it seems to me that the {{{getFreeThreads()}}} method is inherently
+ dangerous, since even though it is internally synchronized, the instant
+ that the method returns to its caller the value could change.
+ 
+ === Possible ways to simplify/repair the synchronization ===
+ 
+ Here are some ideas I've had about possible changes to the synchronization:
+  * I think that the open-coded logic in {{{ClientThread.java}}} which
+    handles creating new {{{Session}}} and {{{DRDAConnThread}}} objects should
+    be moved into the {{{NetworkServerImpl}}} class so that it can be
+    uniformly synchronized with the other logic in that class.
+  * I think that it is confusing to use the {{{runQueue}}} as both a
+    collection object and as a thread wait queue, and it would be simpler and
+    less error-prone to use two separate objects for that.
+  * I think that primitive methods like {{{getThreadList()}}} and
+    {{{getFreeThreads()}}} should be avoided, as they risk exposing too much
+    of the implementation to callers, and allowing race conditions to be
+    accidentally introduced.
+ 
  == Changing the Session Management code ==
  
  I think that there are a number of problems in the Session Management code

Mime
View raw message