From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30576 invoked by alias); 18 Mar 2008 20:12:05 -0000 Received: (qmail 30550 invoked by uid 367); 18 Mar 2008 20:12:05 -0000 Date: Tue, 18 Mar 2008 20:12:00 -0000 Message-ID: <20080318201205.30535.qmail@sourceware.org> From: cagney@sourceware.org To: frysk-cvs@sourceware.org Subject: [SCM] master: Stop Task being created as a Proc; work-around race in Proc.sendRefresh. X-Git-Refname: refs/heads/master X-Git-Reftype: branch X-Git-Oldrev: 2ec8b8634fd4699f0a5d3c602d50369a12e73355 X-Git-Newrev: 60b3e338ecd23f9bf5a2f67a7d7c177f90c640ba Mailing-List: contact frysk-cvs-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: frysk-cvs-owner@sourceware.org Reply-To: frysk@sourceware.org X-SW-Source: 2008-q1/txt/msg00402.txt.bz2 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 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 * 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 * ProcBuilder.java: Use ProcessIdentifier.intValue() not .hashCode(). commit 8b61a34232d3ac1830debe04312916fc6dc9f518 Author: Andrew Cagney 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 * 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 + + * 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 * 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 + + * ProcBuilder.java: Use ProcessIdentifier.intValue() not + .hashCode(). + 2008-03-11 Andrew Cagney * 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