db-derby-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kahat...@apache.org
Subject svn commit: r441802 - in /db/derby/code/trunk/java/drda/org/apache/derby/impl/drda: ClientThread.java NetworkServerControlImpl.java
Date Sat, 09 Sep 2006 15:50:02 GMT
Author: kahatlen
Date: Sat Sep  9 08:50:02 2006
New Revision: 441802

URL: http://svn.apache.org/viewvc?view=rev&rev=441802
Log:
DERBY-1817: Race condition in network server's thread pool

Instead of always putting new sessions in the run queue when there are
free threads, the network server now compares the number of free
threads and the size of the run queue. This is done to prevent the run
queue from growing to a size greater than the number of free
threads. Also, the server now synchronizes on runQueue until the
session has been added to the queue. This is to prevent two threads
from deciding that there are enough free threads and adding the
session to the run queue, when there in fact only were enough free
threads for one of them. With this patch, I am not able to reproduce
DERBY-1757 on platforms where the failure was easily reproduced
before.

Modified:
    db/derby/code/trunk/java/drda/org/apache/derby/impl/drda/ClientThread.java
    db/derby/code/trunk/java/drda/org/apache/derby/impl/drda/NetworkServerControlImpl.java

Modified: db/derby/code/trunk/java/drda/org/apache/derby/impl/drda/ClientThread.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/drda/org/apache/derby/impl/drda/ClientThread.java?view=diff&rev=441802&r1=441801&r2=441802
==============================================================================
--- db/derby/code/trunk/java/drda/org/apache/derby/impl/drda/ClientThread.java (original)
+++ db/derby/code/trunk/java/drda/org/apache/derby/impl/drda/ClientThread.java Sat Sep  9
08:50:02 2006
@@ -47,7 +47,6 @@
 		{
 
 			Socket clientSocket = null;
-			Session clientSession = null;
 
 
 			for (;;)
@@ -84,31 +83,8 @@
 							Integer.toString(connNum));
 
 				//create a new Session for this session
-				// Note that we always re-fetch the tracing
-				// configuration from the parent, because it
-				// may have changed (there are administrative
-				// commands which allow dynamic tracing
-				// reconfiguration).
-				clientSession = new Session(connNum, clientSocket, 
-					parent.getTraceDirectory(),
-					parent.getTraceAll());
+				parent.addSession(connNum, clientSocket);
 
-				//add to Session list
-				parent.addToSessionTable(new Integer(connNum), clientSession);
-
-				//create a new thread for this connection if we need one
-				//and if we are allowed
-				if (parent.getFreeThreads() == 0 && 
-					(parent.getMaxThreads() == 0  || 
-					parent.getThreadList().size() < parent.getMaxThreads()))
-				{
-					DRDAConnThread thread = new DRDAConnThread(clientSession, 
-						parent, timeSlice, parent.getLogConnections());
-					parent.getThreadList().addElement(thread);
-					thread.start();
-				}
-				else //wait for a free thread
-					parent.runQueueAdd(clientSession);
 				}catch (Exception e) {
 					if (e instanceof InterruptedException)
 						return;

Modified: db/derby/code/trunk/java/drda/org/apache/derby/impl/drda/NetworkServerControlImpl.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/drda/org/apache/derby/impl/drda/NetworkServerControlImpl.java?view=diff&rev=441802&r1=441801&r2=441802
==============================================================================
--- db/derby/code/trunk/java/drda/org/apache/derby/impl/drda/NetworkServerControlImpl.java
(original)
+++ db/derby/code/trunk/java/drda/org/apache/derby/impl/drda/NetworkServerControlImpl.java
Sat Sep  9 08:50:02 2006
@@ -3372,14 +3372,58 @@
 
 
 	/**
-	 * Add To Session Table - for use by ClientThread, add a new Session to the sessionTable.
+	 * Add a session - for use by <code>ClientThread</code>. Put the session
+	 * into the session table and the run queue. Start a new
+	 * <code>DRDAConnThread</code> if there are more sessions waiting than
+	 * there are free threads, and the maximum number of threads is not
+	 * exceeded.
 	 *
-	 * @param i	Connection number to register
-	 * @param s	Session to add to the sessionTable
+	 * @param connectionNumber number of connection
+	 * @param clientSocket the socket to read from and write to
 	 */
-	protected void addToSessionTable(Integer i, Session s)
-	{
-		sessionTable.put(i, s);
+	void addSession(int connectionNumber, Socket clientSocket)
+			throws IOException {
+
+		// Note that we always re-fetch the tracing configuration because it
+		// may have changed (there are administrative commands which allow
+		// dynamic tracing reconfiguration).
+		Session session = new Session(connectionNumber, clientSocket,
+									  getTraceDirectory(), getTraceAll());
+
+		sessionTable.put(new Integer(connectionNumber), session);
+
+		// Synchronize on runQueue to prevent other threads from updating
+		// runQueue or freeThreads. Hold the monitor's lock until a thread is
+		// started or the session is added to the queue. If we release the lock
+		// earlier, we might start too few threads (DERBY-1817).
+		synchronized (runQueue) {
+			DRDAConnThread thread = null;
+
+			// try to start a new thread if we don't have enough free threads
+			// to service all sessions in the run queue
+			if (freeThreads <= runQueue.size()) {
+				// Synchronize on threadsSync to ensure that the value of
+				// maxThreads doesn't change until the new thread is added to
+				// threadList.
+				synchronized (threadsSync) {
+					// only start a new thread if we have no maximum number of
+					// threads or the maximum number of threads is not exceeded
+					if ((maxThreads == 0) ||
+							(threadList.size() < maxThreads)) {
+						thread = new DRDAConnThread(session, this,
+													getTimeSlice(),
+													getLogConnections());
+						threadList.add(thread);
+						thread.start();
+					}
+				}
+			}
+
+			// add the session to the run queue if we didn't start a new thread
+			if (thread == null) {
+				runQueueAdd(session);
+			}
+		}
 	}
 
 	/**



Mime
View raw message