From 4557f606c60031b6d7630723e79173d1da340d93 Mon Sep 17 00:00:00 2001 From: stefanbjarni Date: Sat, 23 Apr 2005 00:16:14 +0000 Subject: [PATCH] Debug session thread and socket leak fixed: To reproduce the bug: - Start a PHP debugging session - Open the Debug View - Select the PHP main thread - Terminate the thread with the red square button Bug explanation: The debugging session uses a Java thread to listen on a server socket on port 10001. This thread runs PHPDBGProxy.PHPLoop.run and blocks in ServerSocket.accept. It stays blocked until a debugger engine connects to the port. If the session is terminated before a debugger engine connects, the PHPLoop thread and the server socket are leaked. Relevance: The server socket continues to sit on port 10001 which means later sessions have to go to ports 10002, 10003 etc. This has caused a lot of problems because people fail to get the configuration right the first time, leak the thread and socket, try again and get a session on port 10002, but continue with a ?DBGSESSID=1@host:10001. I think, but I'm not sure, that this leads to the socket write error exception that has been reported in the forums - so that problem may be fixed with this patched (but then again may not be). Bug fix: - The UI thread forcibly unblocks the loop thread by closing the server socket. - The PHPDebugTarget.terminate method is now synchronized and re-entrant, to eliminate a race condition between the UI and loop threads. Two more fixes below. PHPDBGProxy.MapPath: Threw StringIndexOutOfBoundsException when the input string is shorter than the mapped prefix. Patch fixes this by using the String.startsWith method. PHPDBGProxy.pause: Inserted check to prevent a null pointer exception --- .../phpdt/internal/debug/core/PHPDBGProxy.java | 80 +++++++++++--------- .../internal/debug/core/model/PHPDebugTarget.java | 8 ++- 2 files changed, 51 insertions(+), 37 deletions(-) diff --git a/net.sourceforge.phpeclipse.debug.core/src/net/sourceforge/phpdt/internal/debug/core/PHPDBGProxy.java b/net.sourceforge.phpeclipse.debug.core/src/net/sourceforge/phpdt/internal/debug/core/PHPDBGProxy.java index b28f529..0ba0723 100644 --- a/net.sourceforge.phpeclipse.debug.core/src/net/sourceforge/phpdt/internal/debug/core/PHPDBGProxy.java +++ b/net.sourceforge.phpeclipse.debug.core/src/net/sourceforge/phpdt/internal/debug/core/PHPDBGProxy.java @@ -150,39 +150,33 @@ public class PHPDBGProxy { } } - private String MapPath(PHPLineBreakpoint phpLBP) - { - IPath filename; - if (remote) - { - filename = phpLBP.getMarker().getResource().getProjectRelativePath(); - filename = remoteSourcePath.append(filename); - } - else - filename = phpLBP.getMarker().getResource().getLocation(); - String path=filename.toOSString(); - if(pathmap!=null&&remote) - { - java.util.Iterator i=pathmap.keySet().iterator(); - while(i.hasNext()) - { - String k=(String)i.next(); - if(path.substring(0,k.length()).equals(k)) - { - path=pathmap.get(k)+path.substring(k.length()); - break; - } - } - } - if(pathtranslation&&remote) - { - if(path.substring(0,1).equals("/")) - path=path.replace('\\','/'); - else - path=path.replace('/','\\'); + private String MapPath(PHPLineBreakpoint phpLBP) { + IPath filename; + if (remote) { + filename = phpLBP.getMarker().getResource() + .getProjectRelativePath(); + filename = remoteSourcePath.append(filename); + } else + filename = phpLBP.getMarker().getResource().getLocation(); + String path = filename.toOSString(); + if (pathmap != null && remote) { + java.util.Iterator i = pathmap.keySet().iterator(); + while (i.hasNext()) { + String k = (String) i.next(); + if (path.startsWith(k)) { + path = pathmap.get(k) + path.substring(k.length()); + break; + } + } + } + if (pathtranslation && remote) { + if (path.substring(0, 1).equals("/")) + path = path.replace('\\', '/'); + else + path = path.replace('/', '\\'); + } + return path; } - return path; - } public void addBreakpoint(IBreakpoint breakpoint) { if (DBGInt == null) @@ -247,7 +241,12 @@ public class PHPDBGProxy { public void pause() { try { - DBGInt.pauseExecution(); + if (null != DBGInt) + DBGInt.pauseExecution(); + else { + // TODO Make sure the Suspend action is grayed out + // when DBGInt is null + } } catch (IOException e) { PHPDebugCorePlugin.log(e); stop(); @@ -347,8 +346,17 @@ public class PHPDBGProxy { } public synchronized void setShouldStop() { - shouldStop = true; - } + shouldStop = true; + try { + // If the loop thread is blocked on the server socket, + // forcibly unblock it to avoid leaking the thread, + // the socket and the port + closeServerSocket(); + } catch (IOException x) { + // Log this as a warning? + PHPDebugCorePlugin.log(x); + } + } public synchronized void notifyWait() { notify(); @@ -473,4 +481,4 @@ public class PHPDBGProxy { } } } -} \ No newline at end of file +} diff --git a/net.sourceforge.phpeclipse.debug.core/src/net/sourceforge/phpdt/internal/debug/core/model/PHPDebugTarget.java b/net.sourceforge.phpeclipse.debug.core/src/net/sourceforge/phpdt/internal/debug/core/model/PHPDebugTarget.java index 9e5dfb8..c86f72b 100644 --- a/net.sourceforge.phpeclipse.debug.core/src/net/sourceforge/phpdt/internal/debug/core/model/PHPDebugTarget.java +++ b/net.sourceforge.phpeclipse.debug.core/src/net/sourceforge/phpdt/internal/debug/core/model/PHPDebugTarget.java @@ -119,7 +119,13 @@ public class PHPDebugTarget implements IPHPDebugTarget, ILaunchListener, IDebugE return isTerminated; } - public void terminate() { + public synchronized void terminate() { + // This method is synchronized to control a race condition between the + // UI thread that terminates the debugging session, and the slave + // thread that executes PHPLoop.run + if (isTerminated) + // Avoid terminating twice... + return; phpDBGProxy.stop(); this.threads = new PHPThread[0]; isTerminated = true; -- 1.7.1