From: stefanbjarni <stefanbjarni>
Date: Sat, 23 Apr 2005 00:16:14 +0000 (+0000)
Subject: Debug session thread and socket leak fixed:
X-Git-Url: http://secure.phpeclipse.com

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
---

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;