public inbox for frysk-cvs@sourceware.org
help / color / mirror / Atom feed
* [SCM]  master: Stop Task being created as a Proc; work-around race in Proc.sendRefresh.
@ 2008-03-18 20:12 cagney
  0 siblings, 0 replies; only message in thread
From: cagney @ 2008-03-18 20:12 UTC (permalink / raw)
  To: frysk-cvs

The branch, master has been updated
       via  60b3e338ecd23f9bf5a2f67a7d7c177f90c640ba (commit)
       via  8b61a34232d3ac1830debe04312916fc6dc9f518 (commit)
      from  2ec8b8634fd4699f0a5d3c602d50369a12e73355 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email.

- Log -----------------------------------------------------------------
commit 60b3e338ecd23f9bf5a2f67a7d7c177f90c640ba
Author: Andrew Cagney <cagney@redhat.com>
Date:   Tue Mar 18 15:24:57 2008 -0400

    Stop Task being created as a Proc; work-around race in Proc.sendRefresh.
    
    frysk-core/frysk/proc/live/ChangeLog
    2008-03-18  Andrew Cagney  <cagney@redhat.com>
    
    	* TestTaskObserverBlocked.java (BlockedFibonacci): Register the
    	process with StopEventLoopWhenProcTerminated earlier.
    	* LinuxPtraceProc.java: Add more logging.
    	* LinuxPtraceHost.java (requestProc(int,FindProc)): Refresh the
    	proc, not its tasks.
    
    frysk-sys/frysk/sys/proc/ChangeLog
    2008-03-18  Andrew Cagney  <cagney@redhat.com>
    
    	* ProcBuilder.java: Use ProcessIdentifier.intValue() not
    	.hashCode().

commit 8b61a34232d3ac1830debe04312916fc6dc9f518
Author: Andrew Cagney <cagney@redhat.com>
Date:   Tue Mar 18 10:08:40 2008 -0400

    Let state driver code recover from errors; don't attempt local recovery.
    
    frysk-core/frysk/proc/live/ChangeLog
    2008-03-18  Andrew Cagney  <cagney@redhat.com>
    
    	* LinuxPtraceTask.java (sendAttach(), sendDetach(Signal)): Don't
    	try to perform error recovery here.

-----------------------------------------------------------------------

Summary of changes:
 frysk-core/frysk/proc/live/ChangeLog               |   11 ++
 frysk-core/frysk/proc/live/LinuxPtraceHost.java    |   29 ++-
 frysk-core/frysk/proc/live/LinuxPtraceProc.java    |    4 +-
 frysk-core/frysk/proc/live/LinuxPtraceTask.java    |   62 +++----
 .../frysk/proc/live/TestTaskObserverBlocked.java   |  185 +++++++++----------
 frysk-sys/frysk/sys/proc/ChangeLog                 |    5 +
 frysk-sys/frysk/sys/proc/ProcBuilder.java          |    2 +-
 7 files changed, 155 insertions(+), 143 deletions(-)

First 500 lines of diff:
diff --git a/frysk-core/frysk/proc/live/ChangeLog b/frysk-core/frysk/proc/live/ChangeLog
index 1680e6b..e7f386e 100644
--- a/frysk-core/frysk/proc/live/ChangeLog
+++ b/frysk-core/frysk/proc/live/ChangeLog
@@ -1,3 +1,14 @@
+2008-03-18  Andrew Cagney  <cagney@redhat.com>
+
+	* TestTaskObserverBlocked.java (BlockedFibonacci): Register the
+	process with StopEventLoopWhenProcTerminated earlier.
+	* LinuxPtraceProc.java: Add more logging.
+	* LinuxPtraceHost.java (requestProc(int,FindProc)): Refresh the
+	proc, not its tasks.
+
+	* LinuxPtraceTask.java (sendAttach(), sendDetach(Signal)): Don't
+	try to perform error recovery here.
+
 2008-03-17  Andrew Cagney  <cagney@redhat.com>
 
 	* LinuxPtraceTaskState.java (getDestroyed): Delete.
diff --git a/frysk-core/frysk/proc/live/LinuxPtraceHost.java b/frysk-core/frysk/proc/live/LinuxPtraceHost.java
index 8e3a38b..febf665 100644
--- a/frysk-core/frysk/proc/live/LinuxPtraceHost.java
+++ b/frysk-core/frysk/proc/live/LinuxPtraceHost.java
@@ -208,6 +208,7 @@ public class LinuxPtraceHost extends LiveHost {
 	fine.log(this, "requestRefresh");
 	Manager.eventLoop.add(new Event() {
 		public void execute() {
+		    fine.log(LinuxPtraceHost.this, "execute - refresh");
 		    LinuxPtraceHost.this.executeRefresh(knownProcesses,
 							updates);
 		}
@@ -224,22 +225,30 @@ public class LinuxPtraceHost extends LiveHost {
     }
 
     public void requestProc(final int theProcId, final FindProc theFinder) {
+	fine.log(this, "requestProc", theProcId);
 	Manager.eventLoop.add(new Event() {
 		private final ProcessIdentifier pid
 		    = ProcessIdentifierFactory.create(theProcId);
 		private final FindProc finder = theFinder;
 		public void execute() {
-		    // Iterate (build) the /proc tree starting with
-		    // the given procId.
-		    new ProcBuilder() {
-			public void build(ProcessIdentifier pid) {
-			    new ProcChanges().update(pid);
-			}
-		    }.construct(pid);
-		    Proc proc = getProc(pid);
+		    fine.log(LinuxPtraceHost.this, "execute - requestProc",
+			     pid);
+		    // Force an update of the proc in the hosts
+		    // tables; this may or may not succeed.
+		    Proc proc = new ProcChanges().update(pid);
 		    if (proc == null) {
 			finder.procNotFound(pid.intValue());
 		    } else {
+			// FIXME: frysk/5964: This has a race: if a
+			// process part way through an attached clone
+			// gets its task-list refreshed using the
+			// below it will create a detached task for
+			// the clone; when the clone handling code
+			// should/would have created an attached task.
+			// This causes a panic.  The fix is to simply
+			// not call Proc.sendRefresh(); unfortunately
+			// many tests are still relying on this
+			// behaviour.
 			proc.sendRefresh();
 			finder.procFound(proc);
 		    }
@@ -256,7 +265,9 @@ public class LinuxPtraceHost extends LiveHost {
 	fine.log(this, "requestCreateAttachedProc");
 	Manager.eventLoop.add(new Event() {
 		public void execute() {
-		    fine.log(LinuxPtraceHost.this, "executeCreateAttachedProc");
+		    fine.log(LinuxPtraceHost.this,
+			     "execute - requestCreateAttachedProc",
+			     exe);
 		    ProcessIdentifier pid
 			= Fork.ptrace(exe, stdin, stdout, stderr, args);
 		    // See if the Host knows about this task.
diff --git a/frysk-core/frysk/proc/live/LinuxPtraceProc.java b/frysk-core/frysk/proc/live/LinuxPtraceProc.java
index a49b232..8a5b7c2 100644
--- a/frysk-core/frysk/proc/live/LinuxPtraceProc.java
+++ b/frysk-core/frysk/proc/live/LinuxPtraceProc.java
@@ -254,10 +254,10 @@ public class LinuxPtraceProc extends LiveProc {
     /**
      * If it hasn't already been read, read the stat structure.
      */
-    public Stat getStat ()
-    {
+    public Stat getStat() {
 	if (stat == null) {
 	    stat = new Stat().scan(ProcessIdentifierFactory.create(getPid()));
+	    fine.log(this, "getStat", stat);
 	}
 	return stat;
     }
diff --git a/frysk-core/frysk/proc/live/LinuxPtraceTask.java b/frysk-core/frysk/proc/live/LinuxPtraceTask.java
index 0c3a102..2850052 100644
--- a/frysk-core/frysk/proc/live/LinuxPtraceTask.java
+++ b/frysk-core/frysk/proc/live/LinuxPtraceTask.java
@@ -334,46 +334,40 @@ public class LinuxPtraceTask extends LiveTask {
 
     public void sendAttach () {
 	fine.log(this, "sendAttach");
-	try
-	    {
-		Ptrace.attach(tid);
-
-		/*
-		 * XXX: Linux kernel has a 'feature' that if a process is already
-		 * stopped and ptrace requests that it be stopped (again) in order to
-		 * attach to it, the signal (SIGCHLD) notifying frysk of the attach's
-		 * pending waitpid event isn't generated.
-		 */
-
-		/*
-		 * XXX: This line sends another signal to frysk
-		 * notifying about the attach's pending waitpid
-		 * regardless of whether the task is running or
-		 * stopped. This avoids hangs on attaching to a
-		 * stopped process. Bug 3316.
-		 */
-		Signal.CHLD.tkill(frysk.sys.Tid.get());
-	    } catch (Errno.Eperm e) {
+	try {
+	    Ptrace.attach(tid);
+	    
+	    // XXX: Linux kernel has a 'feature' that if a process is
+	    // already stopped and ptrace requests that it be stopped
+	    // (again) in order to attach to it, the signal (SIGCHLD)
+	    // notifying frysk of the attach's pending waitpid event
+	    // isn't generated.  XXX: This line sends another signal
+	    // to frysk notifying about the attach's pending waitpid
+	    // regardless of whether the task is running or
+	    // stopped. This avoids hangs on attaching to a stopped
+	    // process. Bug 3316.
+
+	    Signal.CHLD.tkill(frysk.sys.Tid.get());
+	} catch (Errno.Eperm e) {
+	    // FIXME: Need to propogate this back up to the initiator
+	    // of the attach some how.
 	    fine.log(this, "cannot attach to process", e);
-	    } catch (Errno.Esrch e) {
-		postDisappearedEvent(e);
-	    }
+	}
+	// NOTE: If there's an Errno.Esrch that is allowed to
+	// propogate all the way up to the state engine which will
+	// immediatly execute "disappeared" - since the task is gone
+	// just get the hell out of here.
     }
 
     void sendDetach(Signal sig) {
 	fine.log(this, "sendDetach");
 	clearIsa();
-	try {
-	    if (sig == Signal.STOP) {
-		fine.log(this, "sendDetach/signal STOP");
-		Signal.STOP.tkill(tid);
-		Ptrace.detach(tid, Signal.NONE);
-	    } else {
-		Ptrace.detach(tid, sig);
-	    }
-	} catch (Exception e) {
-	    // Ignore problems trying to detach, most of the time the
-	    // problem is the process has already left the cpu queue
+	if (sig == Signal.STOP) {
+	    fine.log(this, "sendDetach/signal STOP");
+	    Signal.STOP.tkill(tid);
+	    Ptrace.detach(tid, Signal.NONE);
+	} else {
+	    Ptrace.detach(tid, sig);
 	}
     }
 
diff --git a/frysk-core/frysk/proc/live/TestTaskObserverBlocked.java b/frysk-core/frysk/proc/live/TestTaskObserverBlocked.java
index 94bee08..254b4a6 100644
--- a/frysk-core/frysk/proc/live/TestTaskObserverBlocked.java
+++ b/frysk-core/frysk/proc/live/TestTaskObserverBlocked.java
@@ -489,106 +489,97 @@ public class TestTaskObserverBlocked extends TestLib {
     proc.assertSendAddForkWaitForAcks();
   }
 
-  /**
-   * Test that Task.requestAddObserver () can be used to hold multiple tasks at
-   * the spawn point. This creates a program that, in turn, creates lots and
-   * lots of tasks. It then checks that the number of task create and delete
-   * events matches the expected.
-   */
-  abstract class BlockingFibonacci
-      extends TaskObserverBase
-  {
-    static final int fibCount = 10;
-
-    TaskSet parentTasks = new TaskSet();
-
-    TaskSet childTasks = new TaskSet();
-
-    /** Program to run. */
-    abstract String fibonacciProgram ();
-
-    /** Seed the observer. */
-    abstract void addFirstObserver (Task task);
-
-    BlockingFibonacci ()
-    {
-
-      // Compute the expected number of tasks (this includes the
-      // main task).
-      Fibonacci fib = new Fibonacci(fibCount);
-
-      DaemonBlockedAtEntry child = new DaemonBlockedAtEntry(new String[] {
-								fibonacciProgram(),
-								Integer.toString(fibCount) });
-      addFirstObserver(child.getMainTask());
-      child.requestRemoveBlock();
-
-      // An object that, when the child process exits, both sets
-      // a flag to record that event, and requests that the
-      // event loop stop.
-      StopEventLoopWhenProcTerminated childRemoved
-	  = new StopEventLoopWhenProcTerminated(child);
-
-      // Repeatedly run the event loop until the child exits
-      // (every time there is a spawn the event loop will stop).
-      int spawnCount = 0;
-      int loopCount = 0;
-      while (loopCount <= fib.getCallCount() && ! childRemoved.terminated) {
-          loopCount++;
-          assertRunUntilStop("run \"fibonacci\" until stop, number "
-                             + spawnCount + " of " + fib.getCallCount());
-          spawnCount += parentTasks.size();
-          parentTasks.unblock(this).clear();
-          childTasks.unblock(this).clear();
-        }
-
-      // The first task, included in fib.getCallCount() isn't
-      // included in the spawn count.
-      assertEquals("number of times spawnObserver added", fib.getCallCount(),
-                   addedCount());
-      assertEquals("number of times spawnObserver deleted", 0, deletedCount());
-      assertEquals("Number of spawns", fib.getCallCount() - 1, spawnCount);
-      assertTrue("child exited", childRemoved.terminated);
-      assertTrue("at least two iterations of the spawn loop", loopCount > 2);
+    /**
+     * Test that Task.requestAddObserver () can be used to hold
+     * multiple tasks at the spawn point. This creates a program that,
+     * in turn, creates lots and lots of tasks. It then checks that
+     * the number of task create and delete events matches the
+     * expected.
+     */
+    private abstract class BlockingFibonacci extends TaskObserverBase {
+	static final int fibCount = 10;
+
+	TaskSet parentTasks = new TaskSet();
+	
+	TaskSet childTasks = new TaskSet();
+	
+	/** Program to run. */
+	abstract String fibonacciProgram ();
+	
+	/** Seed the observer. */
+	abstract void addFirstObserver (Task task);
+	
+	BlockingFibonacci() {
+
+	    // Compute the expected number of tasks (this includes the
+	    // main task).
+	    Fibonacci fib = new Fibonacci(fibCount);
+	    
+	    DaemonBlockedAtEntry child = new DaemonBlockedAtEntry(new String[] {
+		    fibonacciProgram(),
+		    Integer.toString(fibCount)
+		});
+	    // An object that, when the child process exits, both sets
+	    // a flag to record that event, and requests that the
+	    // event loop stop.
+	    StopEventLoopWhenProcTerminated childRemoved
+		= new StopEventLoopWhenProcTerminated(child);
+
+	    addFirstObserver(child.getMainTask());
+	    child.requestRemoveBlock();
+	    
+	    // Repeatedly run the event loop until the child exits
+	    // (every time there is a spawn the event loop will stop).
+	    int spawnCount = 0;
+	    int loopCount = 0;
+	    while (loopCount <= fib.getCallCount() && ! childRemoved.terminated) {
+		loopCount++;
+		assertRunUntilStop("run \"fibonacci\" until stop, number "
+				   + spawnCount + " of " + fib.getCallCount());
+		spawnCount += parentTasks.size();
+		parentTasks.unblock(this).clear();
+		childTasks.unblock(this).clear();
+	    }
+	    
+	    // The first task, included in fib.getCallCount() isn't
+	    // included in the spawn count.
+	    assertEquals("number of times spawnObserver added", fib.getCallCount(),
+			 addedCount());
+	    assertEquals("number of times spawnObserver deleted", 0, deletedCount());
+	    assertEquals("Number of spawns", fib.getCallCount() - 1, spawnCount);
+	    assertTrue("child exited", childRemoved.terminated);
+	    assertTrue("at least two iterations of the spawn loop", loopCount > 2);
+	}
     }
-  }
 
-  /**
-   * Check that a program rapidly cloning can be stopped and started at the
-   * cline points.
-   */
-  public void testBlockingFibonacciClone ()
-  {
-    class CloneFibonacci
-        extends BlockingFibonacci
-        implements TaskObserver.Cloned
-    {
-      public Action updateClonedParent (Task parent, Task offspring)
-      {
-        parentTasks.add(parent);
-        return Action.BLOCK;
-      }
-
-      public Action updateClonedOffspring (Task parent, Task offspring)
-      {
-        offspring.requestAddClonedObserver(this);
-        childTasks.add(offspring);
-        Manager.eventLoop.requestStop();
-        return Action.BLOCK;
-      }
-
-      void addFirstObserver (Task task)
-      {
-        task.requestAddClonedObserver(this);
-      }
-
-      String fibonacciProgram ()
-      {
-	  return getExecPath ("funit-fib-clone");
-      }
+    /**
+     * Check that a program rapidly cloning can be stopped and started
+     * at the cline points.
+     */
+    public void testBlockingFibonacciClone() {
+	class CloneFibonacci
+	    extends BlockingFibonacci
+	    implements TaskObserver.Cloned
+	{
+	    public Action updateClonedParent(Task parent, Task offspring) {
+		parentTasks.add(parent);
+		return Action.BLOCK;
+	    }
+	    public Action updateClonedOffspring(Task parent, Task offspring) {
+		offspring.requestAddClonedObserver(this);
+		childTasks.add(offspring);
+		Manager.eventLoop.requestStop();
+		return Action.BLOCK;
+	    }
+	    void addFirstObserver(Task task) {
+		task.requestAddClonedObserver(this);
+	    }
+	    String fibonacciProgram() {
+		return getExecPath ("funit-fib-clone");
+	    }
+	}
+	new CloneFibonacci();
     }
-    new CloneFibonacci();
-  }
 
   /**
    * Check that a program rapidly cloning can be stopped and started at the
diff --git a/frysk-sys/frysk/sys/proc/ChangeLog b/frysk-sys/frysk/sys/proc/ChangeLog
index 39f22c8..c7ddaab 100644
--- a/frysk-sys/frysk/sys/proc/ChangeLog
+++ b/frysk-sys/frysk/sys/proc/ChangeLog
@@ -1,3 +1,8 @@
+2008-03-18  Andrew Cagney  <cagney@redhat.com>
+
+	* ProcBuilder.java: Use ProcessIdentifier.intValue() not
+	.hashCode().
+
 2008-03-11  Andrew Cagney  <cagney@redhat.com>
 
 	* cni/ProcBuilder.cxx (ProcBuilder::scan): Add warnings when PID
diff --git a/frysk-sys/frysk/sys/proc/ProcBuilder.java b/frysk-sys/frysk/sys/proc/ProcBuilder.java
index 9f107ce..b99bb81 100644
--- a/frysk-sys/frysk/sys/proc/ProcBuilder.java
+++ b/frysk-sys/frysk/sys/proc/ProcBuilder.java
@@ -58,7 +58,7 @@ public abstract class ProcBuilder {
      * "finally" to ensure that the directory is always closed.
      */
     public final boolean construct(ProcessIdentifier pid) {
-	return construct(pid.hashCode ());
+	return construct(pid.intValue());
     }
     private boolean construct(int pid) {
 	RawData dir = open(pid);


hooks/post-receive
--
frysk system monitor/debugger


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2008-03-18 20:12 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-03-18 20:12 [SCM] master: Stop Task being created as a Proc; work-around race in Proc.sendRefresh cagney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).