From: Ron S. <ron...@ya...> - 2006-03-21 18:36:56
|
User: rsigal Date: 06/03/21 13:36:40 Modified: src/main/org/jboss/remoting/transport/socket ServerThread.java SocketServerInvoker.java Log: JBREM-334: Out of memory error A race between SocketServerInvoker.cleanup() and ServerThread.run() leads occasionally to ServerThreads being left alive, which causes a memory leak. Parts of these two methods have been synchronized. Revision Changes Path 1.21 +44 -17 JBossRemoting/src/main/org/jboss/remoting/transport/socket/ServerThread.java (In the diff below, changes in quantity of whitespace are not shown.) Index: ServerThread.java =================================================================== RCS file: /cvsroot/jboss/JBossRemoting/src/main/org/jboss/remoting/transport/socket/ServerThread.java,v retrieving revision 1.20 retrieving revision 1.21 diff -u -b -r1.20 -r1.21 --- ServerThread.java 21 Mar 2006 07:37:44 -0000 1.20 +++ ServerThread.java 21 Mar 2006 18:36:40 -0000 1.21 @@ -51,7 +51,7 @@ * * @author <a href="mailto:bi...@jb...">Bill Burke</a> * @author <a href="mailto:to...@jb...">Tom Elrod</a> - * @version $Revision: 1.20 $ + * @version $Revision: 1.21 $ */ public class ServerThread extends Thread { @@ -237,32 +237,59 @@ while(true) { dorun(); - if(shutdown) - { + +/* + * The following code has been changed to eliminate a race condition with SocketServerInvoker.cleanup(). + * A ServerThread can shutdown for two reasons: + * + * 1. the client shuts down, and + * 2. the server shuts down. + * + * If both occur around the same time, a problem arises. If a ServerThread starts to shut + * down because the client shut down, it will test shutdown, and if it gets to the test + * before SocketServerInvoker.cleanup() calls ServerThread.stop() to set shutdown to true, it + * will return itself to threadpool. If it moves from clientpool to threadpool at just the + * right time, SocketServerInvoker could miss it in both places and never call stop(), leaving + * it alive, resulting in a memory leak. The solution is to synchronize parts of + * ServerThread.run() and SocketServerInvoker.cleanup() so that they interact atomically. + */ synchronized(clientpool) { - clientpool.remove(this); - } + synchronized(threadpool) + { + if(shutdown) + { + invoker = null; return; // exit thread } else { - synchronized(this) - { - synchronized(clientpool) - { - synchronized(threadpool) - { clientpool.remove(this); threadpool.add(this); Thread.interrupted(); // clear any interruption so that we can be pooled. clientpool.notify(); } } + } + + synchronized(this) + { + try + { log.debug("begin thread wait"); this.wait(); log.debug("WAKEUP in SERVER THREAD"); } + catch (InterruptedException e) + { + if (shutdown) + { + invoker = null; + return; // exit thread + } + + throw e; + } } } } 1.19 +33 -19 JBossRemoting/src/main/org/jboss/remoting/transport/socket/SocketServerInvoker.java (In the diff below, changes in quantity of whitespace are not shown.) Index: SocketServerInvoker.java =================================================================== RCS file: /cvsroot/jboss/JBossRemoting/src/main/org/jboss/remoting/transport/socket/SocketServerInvoker.java,v retrieving revision 1.18 retrieving revision 1.19 diff -u -b -r1.18 -r1.19 --- SocketServerInvoker.java 14 Mar 2006 18:32:52 -0000 1.18 +++ SocketServerInvoker.java 21 Mar 2006 18:36:40 -0000 1.19 @@ -42,7 +42,7 @@ * * @author <a href="mailto:jh...@vo...">Jeff Haynie</a> * @author <a href="mailto:tom...@jb...">Tom Elrod</a> - * @version $Revision: 1.18 $ + * @version $Revision: 1.19 $ * @jmx:mbean */ public class SocketServerInvoker extends ServerInvoker implements Runnable, SocketServerInvokerMBean @@ -234,27 +234,41 @@ } } } + +/* + * The following code has been changed to avoid a race condition with ServerThread.run() which + * can result in leaving ServerThreads alive, which causes a memory leak. + */ if (clientpool != null) { + synchronized (clientpool) + { Set svrThreads = clientpool.getContents(); Iterator itr = svrThreads.iterator(); + while(itr.hasNext()) { Object o = itr.next(); ServerThread st = (ServerThread) o; st.shutdown(); } + clientpool.flush(); clientpool.stop(); - } + if (threadpool != null) { + synchronized(threadpool) + { for(int i = 0; i < threadpool.size(); i++) { ServerThread thread = (ServerThread) threadpool.removeFirst(); thread.shutdown(); } } + } + } + } try { |