* Forward of blocked observer discussion
@ 2006-10-12 19:15 Nurdin Premji
2006-10-12 19:24 ` Nurdin Premji
2006-10-12 21:38 ` Andrew Cagney
0 siblings, 2 replies; 9+ messages in thread
From: Nurdin Premji @ 2006-10-12 19:15 UTC (permalink / raw)
To: frysk
The following is a pasted together transcript of whats going on with
blocked and Step/InstructionObservers.
Mark Wielaard wrote:
Implementing BlockObserver and StepObserver
Andrew Cagney wrote:
> The two are being merged? unblocking a block observer has the effect
of
> a Step?
Mark
No, since I believe a BlockObserver (now called TaskObserver.Blocked) is
useful on its own. But the implementation will be almost identical since
I am merging the TaskState code paths that make sure a Task is
(temporarily) suspended since TaskObserver.Syscall and TaskObserver.Code
also need that (and currently use slightly different code to achieve
that functionality). This is the api doc that I wrote for it:
/**
+ * Interface used to notify that a Task has been blocked for now.
+ * When the <code>updateBlocked</code> method is triggered it is
+ * guaranteed to Task has (temporarily) suspended
+ * execution. <code>updateBlocked</code> will be called at most
+ * once. If <code>Action.CONTINUE</code> is returned or after
+ * <code>requestUnblock()</code> is called on the Task the
+ * observer is removed from the task and will not be triggered
+ * unless it is explictily readded. To continiously monitor a
+ * running Task see the <code>Instruction</code> interface.
+ */
+ public interface Blocked
+ extends TaskObserver
+ {
+ /**
+ * The task has executed one instruction. Return Action.BLOCK
+ * to block the task's further execution.
+ */
+ Action updateBlocked (Task task);
+ }
The TaskObserver.Instruction (which already existed, so I took that name
instead of StepObserver) does the same thing as Blocked, but stays
around. So if there are no other blocking observers it will put the Task
in Running state and almost immediately call updateExecuted(). Otherwise
it works the same as Blocked:
/**
* Interface used to notify that a Task has executed a single
* instruction.
*/
public interface Instruction
extends TaskObserver
{
/**
* The task has executed one instruction. Return Action.BLOCK
* to block the task's further execution.
*/
Action updateExecuted (Task task);
}
Andrew Cagney wrote:
> Mark, there's too much redundancy here - we should be after a
relatively
> canonical set of control mechanisms - and redundancy means writing
twice
> as much testing framework for what are effectively two mechanisms.
> Having an observer remove itself when there's an unblock, is just
> wierd.
Mark
Agreed, but if you control a Task through Observers only then there is
not much choice for 'temporarily holding a Task'. And I believe that
functionality on itself is useful.
> Does that occure codintional on returning CONTINUE vs being sent
> an unblock, ...., again too confusing for no value.
Yes. But if you think that is confusing we can just hold the task till
it is explicitly deleted.
> Sami, Adam, and I discussed this at length
Is that discussion documented somewhere?
> and concluded just the blocking observer, where an unblock triggered
> an attempt at a single-step is all that was required.
So you only want the TaskObserver.Instruction? What about operations,
like getting the current stack frame for a stack trace or profile
sampling that don't need the extra complexity of stepping?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Forward of blocked observer discussion
2006-10-12 19:15 Forward of blocked observer discussion Nurdin Premji
@ 2006-10-12 19:24 ` Nurdin Premji
2006-10-16 15:05 ` Mark Wielaard
2006-10-12 21:38 ` Andrew Cagney
1 sibling, 1 reply; 9+ messages in thread
From: Nurdin Premji @ 2006-10-12 19:24 UTC (permalink / raw)
To: mark, frysk
Mark,
So what I want in terms of a blocked observer are two things.
#1 A replacement for Mike's ProcAttachedObserver that looks fairly the
same except all the work for existing tasks is called after the proc has
been completely blocked (rather than in updateAttached as it currently
is).
So you would write something like:
Proc proc = process,
new ProcBlockObserver(proc, new Proc.BlockObserver() {
existingTask(Task task)
{ do stuff}
});
and the process will be blocked and then the observer will run through
all the tasks and perform the stuff from existing task.
(Possibly automatically removing the observer once all the tasks have
been processed, but I don't know, someone might want to reuse the
observer, maybe by calling a function like ProcBlockedObserver.rerun or
something like that)
#2 A mechanism to fix ProcTasksObserver so that when the observer is
added to a process, that process is immediately stopped, there are many
race conditions here.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Forward of blocked observer discussion
2006-10-12 19:15 Forward of blocked observer discussion Nurdin Premji
2006-10-12 19:24 ` Nurdin Premji
@ 2006-10-12 21:38 ` Andrew Cagney
2006-10-12 23:43 ` Mark Wielaard
2006-10-13 19:49 ` Sami Wagiaalla
1 sibling, 2 replies; 9+ messages in thread
From: Andrew Cagney @ 2006-10-12 21:38 UTC (permalink / raw)
To: Mark Wielaard; +Cc: frysk
> /**
> If <code>Action.CONTINUE</code> is returned or after
> + * <code>requestUnblock()</code> is called on the Task the
> + * observer is removed from the task and will not be triggered
> + * unless it is explictily readded. To continiously monitor a
> + * running Task see the <code>Instruction</code> interface.
> + */
Mark,
This adds an obscure side effect to Action.CONTINUE, is inconsistent
with every other observer, and is redundant. Any observer, not just a
blocking observer, needing to both unblock and remove itself can just
request its removal. The state machine needs to then directly handle
that, but as npremji recently discovered, and is now working on, it
isn't reliable.
Also remember, as you/I previously discussed, and similarly I sami and
adam discussed, the blocking and instruction observers have too much
functional overlap, making one redundant. BTW, Sami/Adam preferred to
see the observer called something like Blocking rather than Instruction
as that better reflected its behavior.
Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Forward of blocked observer discussion
2006-10-12 21:38 ` Andrew Cagney
@ 2006-10-12 23:43 ` Mark Wielaard
2006-10-13 14:31 ` Andrew Cagney
2006-10-13 19:49 ` Sami Wagiaalla
1 sibling, 1 reply; 9+ messages in thread
From: Mark Wielaard @ 2006-10-12 23:43 UTC (permalink / raw)
To: Andrew Cagney; +Cc: frysk
Hi Andrew,
On Thu, 2006-10-12 at 17:38 -0400, Andrew Cagney wrote:
> > /**
> > If <code>Action.CONTINUE</code> is returned or after
> > + * <code>requestUnblock()</code> is called on the Task the
> > + * observer is removed from the task and will not be triggered
> > + * unless it is explictily readded. To continiously monitor a
> > + * running Task see the <code>Instruction</code> interface.
> > + */
> This adds an obscure side effect to Action.CONTINUE, is inconsistent
> with every other observer, and is redundant. Any observer, not just a
> blocking observer, needing to both unblock and remove itself can just
> request its removal.
Yes. But if you think that is confusing we can just hold the task till
it is explicitly deleted. The only issue is that with a Blocking
observer it is unclear what the difference between Action.CONTINUE and
Action.BLOCK is. We can just decide there isn't a difference for
Blocked.
> The state machine needs to then directly handle
> that, but as npremji recently discovered, and is now working on, it
> isn't reliable.
What isn't reliable?
> Also remember, as you/I previously discussed, and similarly I sami and
> adam discussed, the blocking and instruction observers have too much
> functional overlap, making one redundant.
At our last meeting we discussed adding two separate observers. Since it
was thought that adding a task blocking observer would be easier to add
first and that we could then add a stepping instruction observer next.
And because they are slightly different/have different roles.
As it turned out the most work was actually adapting the TaskState
machine and the way you add a TaskObservation to make it easy to request
a temporary suspend from the Task to change any aspects of the Task. Now
adding any observer that (temporarily) suspends, whatever its semantics
should be easy.
> BTW, Sami/Adam preferred to
> see the observer called something like Blocking rather than Instruction
> as that better reflected its behavior.
The Instruction observer (for which I just committed a first version) is
different from a Blocking observer. The Instruction observer gives
updates whenever a Task executes an instruction. The Blocked observer
would give an update as soon as the Task is suspended (or immediately
when the Task is already blocked).
Note that for reliably knowing whether a Task is suspended or not the
Instruction observer alone isn't enough since it won't trigger if the
Task is already blocked.
Cheers,
Mark
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Forward of blocked observer discussion
2006-10-12 23:43 ` Mark Wielaard
@ 2006-10-13 14:31 ` Andrew Cagney
2006-10-15 2:24 ` Mark Wielaard
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Cagney @ 2006-10-13 14:31 UTC (permalink / raw)
To: Mark Wielaard; +Cc: frysk
Mark Wielaard wrote:
> The Instruction observer (for which I just committed a first version) is
> different from a Blocking observer. The Instruction observer gives
> updates whenever a Task executes an instruction. The Blocked observer
> would give an update as soon as the Task is suspended (or immediately
> when the Task is already blocked).
>
>
Right, and I was also given the argument that the step observer should
notify on first stopping attach, and not delay until an instruction has
been apparently executed. I think we should go with that (which makes
blocking completely redundant) until we've counter evidence from the UI
developers.
Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Forward of blocked observer discussion
2006-10-12 21:38 ` Andrew Cagney
2006-10-12 23:43 ` Mark Wielaard
@ 2006-10-13 19:49 ` Sami Wagiaalla
2006-10-15 2:38 ` Mark Wielaard
1 sibling, 1 reply; 9+ messages in thread
From: Sami Wagiaalla @ 2006-10-13 19:49 UTC (permalink / raw)
To: frysk
>
> This adds an obscure side effect to Action.CONTINUE, is inconsistent
> with every other observer, and is redundant. Any observer, not just
> a blocking observer, needing to both unblock and remove itself can
> just request its removal. The state machine needs to then directly
> handle that, but as npremji recently discovered, and is now working
> on, it isn't reliable.
Agreed. It will be hard to explain to the user why Action.CONTINUE
results in observer removal, especially if it is called instruction
observer.
>
> Also remember, as you/I previously discussed, and similarly I sami and
> adam discussed, the blocking and instruction observers have too much
> functional overlap, making one redundant.
The biggest difference between Instruction and Blocking observer is that
return Action.CONTINUE results in a instruction step for one and
resuming the process for the other, not weather it removes itself or not.
Despite the obvious functionality overlap both observers are need (at
least both names are needed). It is hard to explain to a user who wants
to block a task why they have to use an observer called
InstructionObserver, and it will also be hard to explain to a user who
wants to step a task why they have to use an observer called BlockObserver.
Why not have one be a client of the other in the same way that
TasksObserver is a client of ForkObserver and CloneObserver ? Avoiding
code and test redundancy. Or maybe just add Action.STEP :D
> BTW, Sami/Adam preferred to see the observer called something like
> Blocking rather than Instruction as that better reflected its behavior.
Yup. We wanted to use it for blocking, we didnt have stepping in mind.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Forward of blocked observer discussion
2006-10-13 14:31 ` Andrew Cagney
@ 2006-10-15 2:24 ` Mark Wielaard
0 siblings, 0 replies; 9+ messages in thread
From: Mark Wielaard @ 2006-10-15 2:24 UTC (permalink / raw)
To: Andrew Cagney; +Cc: frysk
[-- Attachment #1: Type: text/plain, Size: 1144 bytes --]
Hi Andrew,
On Fri, 2006-10-13 at 10:32 -0400, Andrew Cagney wrote:
> the argument that the step observer should
> notify on first stopping attach, and not delay until an instruction has
> been apparently executed. I think we should go with that (which makes
> blocking completely redundant) until we've counter evidence from the UI
> developers.
OK. Lets go with that for now. The attached patch implements this. The
actual code to implement this is just 5 lines. Most of the patch is
documentation for the new semantics and a testcase for various
combinations of adding/removing/unblocking (the test uncovered a bug in
deletion, which is now also fixed).
2006-10-14 Mark Wielaard <mark@klomp.org>
* Proc.java (requestAddInstructionObserver): Call updateExecuted()
when observation is added.
(requestDeleteInstructionObserver): Delete don't add the observer.
* TaskObserver.java (Instruction): Update documentation.
* TaskState.java (Running.handleStoppedEvent): Check for blockers.
* TestLib (AckDaemonProcess(boolean)): New constructor.
* TestTaskObserverInstruction.java: New tests.
Committed,
Mark
[-- Attachment #2: step-blocks.patch --]
[-- Type: text/x-patch, Size: 10360 bytes --]
Index: frysk-core/frysk/proc/Proc.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/Proc.java,v
retrieving revision 1.86
diff -u -r1.86 Proc.java
--- frysk-core/frysk/proc/Proc.java 13 Oct 2006 15:40:31 -0000 1.86
+++ frysk-core/frysk/proc/Proc.java 15 Oct 2006 02:16:26 -0000
@@ -647,6 +647,11 @@
// to make sure the Task is stopped so we can send it a step
// instruction or, when deleted, start resuming the process
// normally.
+
+ // We do want an explicit updateExecuted() call, after adding
+ // the observer, but while still suspended. This is done by
+ // overriding the add() method in the TaskObservation
+ // below. No such action is required on deletion.
}
}
@@ -675,6 +680,18 @@
{
return task.instructionObservers.numberOfObservers() == 0;
}
+
+ // Makes sure that the observer is properly added and then,
+ // while the Task is still suspended, updateExecuted() is
+ // called. Giving the observer a chance to inspect and
+ // possibly block the Task.
+ public void add()
+ {
+ super.add();
+ TaskObserver.Instruction i = (TaskObserver.Instruction) observer;
+ if (i.updateExecuted(task) == Action.BLOCK)
+ task.blockers.add(observer);
+ }
};
Manager.eventLoop.add(to);
}
@@ -695,7 +712,7 @@
{
public void execute ()
{
- handleAddObservation (this);
+ newState = oldState().handleDeleteObservation(Proc.this, this);
}
public boolean needsSuspendedAction()
Index: frysk-core/frysk/proc/TaskObserver.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/TaskObserver.java,v
retrieving revision 1.18
diff -u -r1.18 TaskObserver.java
--- frysk-core/frysk/proc/TaskObserver.java 12 Oct 2006 22:05:43 -0000 1.18
+++ frysk-core/frysk/proc/TaskObserver.java 15 Oct 2006 02:16:26 -0000
@@ -177,16 +177,28 @@
/**
* Interface used to notify that a Task has executed a single
- * instruction. <code>updateExecuted</code> is called as soon after
- * the Instruction observer is added to the Task and the Task starts
- * running again (isn't blocked or suspended).
+ * instruction. <code>updateExecuted</code> is called as soon as
+ * the Instruction observer is added to the Task. And whenever the
+ * Task starts running again (isn't blocked or suspended) it will
+ * be called on each instruction being executed.
+ * <p>
+ * This TaskObserver can also be used for executing code that
+ * needs the Task to be (temporarily) blocked or suspended as soon
+ * as possible. <code>updateExecuted()</code> will be called as
+ * soon as this observer has been properly added, and at that time
+ * the Task is suspended to make it possible to inspect the Task
+ * state. If no other action is request, the method can then just
+ * delete the observer from the Task again.
*/
public interface Instruction
extends TaskObserver
{
/**
- * The task has executed one instruction. Return Action.BLOCK
- * to block the task's further execution.
+ * The task has started executing or has executed another
+ * instruction. Return Action.BLOCK to block the task's
+ * further execution. When Action.CONTINUE is returned
+ * this method will be called as soon as one instruction
+ * has been executed.
*/
Action updateExecuted (Task task);
}
Index: frysk-core/frysk/proc/TaskState.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/TaskState.java,v
retrieving revision 1.122
diff -u -r1.122 TaskState.java
--- frysk-core/frysk/proc/TaskState.java 12 Oct 2006 22:05:43 -0000 1.122
+++ frysk-core/frysk/proc/TaskState.java 15 Oct 2006 02:16:26 -0000
@@ -762,6 +762,10 @@
it.remove();
}
+ // the observation.add() could have added a block.
+ if (task.blockers.size () > 0)
+ return blockedContinue();
+
// See how to continue depending on the kind of observers.
if (task.instructionObservers.numberOfObservers() > 0)
{
Index: frysk-core/frysk/proc/TestLib.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/TestLib.java,v
retrieving revision 1.109
diff -u -r1.109 TestLib.java
--- frysk-core/frysk/proc/TestLib.java 6 Oct 2006 20:57:07 -0000 1.109
+++ frysk-core/frysk/proc/TestLib.java 15 Oct 2006 02:16:26 -0000
@@ -599,6 +599,10 @@
{
super ();
}
+ AckDaemonProcess (boolean busy)
+ {
+ super (busy);
+ }
AckDaemonProcess (int count)
{
super (count);
Index: frysk-core/frysk/proc/TestTaskObserverInstruction.java
===================================================================
RCS file: frysk-core/frysk/proc/TestTaskObserverInstruction.java
diff -N frysk-core/frysk/proc/TestTaskObserverInstruction.java
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ frysk-core/frysk/proc/TestTaskObserverInstruction.java 15 Oct 2006 02:16:26 -0000
@@ -0,0 +1,145 @@
+// This file is part of the program FRYSK.
+//
+// Copyright 2006, Red Hat Inc.
+//
+// FRYSK is free software; you can redistribute it and/or modify it
+// under the terms of the GNU General Public License as published by
+// the Free Software Foundation; version 2 of the License.
+//
+// FRYSK is distributed in the hope that it will be useful, but
+// WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+// General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with FRYSK; if not, write to the Free Software Foundation,
+// Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+//
+// In addition, as a special exception, Red Hat, Inc. gives You the
+// additional right to link the code of FRYSK with code not covered
+// under the GNU General Public License ("Non-GPL Code") and to
+// distribute linked combinations including the two, subject to the
+// limitations in this paragraph. Non-GPL Code permitted under this
+// exception must only link to the code of FRYSK through those well
+// defined interfaces identified in the file named EXCEPTION found in
+// the source code files (the "Approved Interfaces"). The files of
+// Non-GPL Code may instantiate templates or use macros or inline
+// functions from the Approved Interfaces without causing the
+// resulting work to be covered by the GNU General Public
+// License. Only Red Hat, Inc. may make changes or additions to the
+// list of Approved Interfaces. You must obey the GNU General Public
+// License in all respects for all of the FRYSK code and other code
+// used in conjunction with FRYSK except the Non-GPL Code covered by
+// this exception. If you modify this file, you may extend this
+// exception to your version of the file, but you are not obligated to
+// do so. If you do not wish to provide this exception without
+// modification, you must delete this exception statement from your
+// version and license this file solely under the GPL without
+// exception.
+
+package frysk.proc;
+
+public class TestTaskObserverInstruction extends TestLib
+{
+ public void testInstruction()
+ {
+ // We want a busy child, because we are going to follow its steps.
+ Child child = new AckDaemonProcess(true);
+ Task task = child.findTaskUsingRefresh (true);
+
+ InstructionObserver instr1 = new InstructionObserver();
+
+ task.requestAddInstructionObserver(instr1);
+ assertRunUntilStop("attach then block");
+
+ assertFalse("deleted", instr1.deleted);
+ assertTrue("added", instr1.added);
+ assertEquals("hit", 1, instr1.hit);
+
+ task.requestUnblock(instr1);
+ assertRunUntilStop("unblock self and hit");
+
+ assertFalse("deleted", instr1.deleted);
+ assertTrue("added", instr1.added);
+ assertEquals("hit", 2, instr1.hit);
+
+ InstructionObserver instr2 = new InstructionObserver();
+
+ task.requestAddInstructionObserver(instr2);
+ assertRunUntilStop("attach while blocked");
+
+ assertFalse("deleted 1", instr1.deleted);
+ assertTrue("added 1", instr1.added);
+ assertEquals("hit 1", 2, instr1.hit);
+
+ assertFalse("deleted 2", instr2.deleted);
+ assertTrue("added 2", instr2.added);
+ assertEquals("hit 2", 1, instr2.hit);
+
+ task.requestUnblock(instr1);
+ task.requestUnblock(instr2);
+ assertRunUntilStop("unblock both");
+
+ assertFalse("deleted both 1", instr1.deleted);
+ assertTrue("added both 1", instr1.added);
+ assertEquals("hit both 1", 3, instr1.hit);
+
+ assertFalse("deleted both 2", instr2.deleted);
+ assertTrue("added both 2", instr2.added);
+ assertEquals("hit both 2", 2, instr2.hit);
+
+ // XXX - unblock needed?
+ task.requestUnblock(instr1);
+ task.requestDeleteInstructionObserver(instr1);
+ task.requestUnblock(instr2);
+ assertRunUntilStop("delete and unblock");
+
+ assertTrue("deleted delete and unblock 1", instr1.deleted);
+ assertTrue("added delete and unblock 1", instr1.added);
+ assertEquals("hit delete and unblock 1", 3, instr1.hit);
+
+ assertFalse("deleted delete and unblock 2", instr2.deleted);
+ assertTrue("added delete and unblock 2", instr2.added);
+ assertEquals("hit delete and unblock 2", 3, instr2.hit);
+
+ task.requestDeleteInstructionObserver(instr2);
+ task.requestAddInstructionObserver(instr1);
+ assertRunUntilStop("delete and add");
+
+ assertTrue("added delete and add 1", instr1.added);
+ assertEquals("hit delete and add 1", 4, instr1.hit);
+
+ assertTrue("deleted delete and add 2", instr2.deleted);
+ assertEquals("hit delete and add 2", 3, instr2.hit);
+ }
+
+ static class InstructionObserver implements TaskObserver.Instruction
+ {
+ boolean added;
+ boolean deleted;
+
+ int hit;
+
+ public Action updateExecuted(Task task)
+ {
+ hit++;
+ Manager.eventLoop.requestStop ();
+ return Action.BLOCK;
+ }
+
+ public void addedTo(Object o)
+ {
+ added = true;
+ }
+
+ public void deletedFrom(Object o)
+ {
+ deleted = true;
+ }
+
+ public void addFailed (Object o, Throwable w)
+ {
+ fail("add to " + o + " failed, because " + w);
+ }
+ }
+}
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Forward of blocked observer discussion
2006-10-13 19:49 ` Sami Wagiaalla
@ 2006-10-15 2:38 ` Mark Wielaard
0 siblings, 0 replies; 9+ messages in thread
From: Mark Wielaard @ 2006-10-15 2:38 UTC (permalink / raw)
To: Sami Wagiaalla; +Cc: frysk
Hi Sami,
On Fri, 2006-10-13 at 13:49 -0600, Sami Wagiaalla wrote:
> > This adds an obscure side effect to Action.CONTINUE, is inconsistent
> > with every other observer, and is redundant. Any observer, not just
> > a blocking observer, needing to both unblock and remove itself can
> > just request its removal. The state machine needs to then directly
> > handle that, but as npremji recently discovered, and is now working
> > on, it isn't reliable.
> Agreed. It will be hard to explain to the user why Action.CONTINUE
> results in observer removal, especially if it is called instruction
> observer.
OK, there is value in keeping things generic.
> > Also remember, as you/I previously discussed, and similarly I sami and
> > adam discussed, the blocking and instruction observers have too much
> > functional overlap, making one redundant.
> The biggest difference between Instruction and Blocking observer is that
> return Action.CONTINUE results in a instruction step for one and
> resuming the process for the other, not weather it removes itself or not.
Yes. The only "problem" with just resuming after the hit is that it is a
"one time trigger" observer. Which means it isn't really needed after a
continue. Removing it would just be a convenience to the user. I imagine
that not doing that might lead to some leaks since people will forget to
remove when it has done its task. So, in that sense having it keep
triggering on a CONTINUE is a good thing, then you notice it is around
and you can decide whether you need it or not (by immediately deleting
it after it has done its task).
> Despite the obvious functionality overlap both observers are need (at
> least both names are needed). It is hard to explain to a user who wants
> to block a task why they have to use an observer called
> InstructionObserver, and it will also be hard to explain to a user who
> wants to step a task why they have to use an observer called BlockObserver.
For now I changed the semantics of the TaskObserver.Instruction to be
called "whenever it is at an instruction", which is obviously always.
That means it will immediately trigger, whether the Task is running or
blocked already and then you can decide to keep going, with the observer
in place, which will then trigger as soon as the Task runs another
instruction, or you can block it.
So removing it after a block or on the first hit gives you a temp block,
keeping it gives you a stepping blocker that just keeps hitting on each
Instruction.
> Why not have one be a client of the other in the same way that
> TasksObserver is a client of ForkObserver and CloneObserver ? Avoiding
> code and test redundancy.
I am not completely sure how you are seeing this. Since TaskObservers
are just interfaces it is not completely trivial to extend one to
provide a little additional functionality (you will have to add a full
accompanying TaskObservation and add support for adding that in Proc
whenever you add your derived TaskObserver interface).
But you can obviously build a "higher-order" TaskObservation on top of
the Instruction observer that does just a one time block. I didn't build
it yet, but that is really just what I originally described, a one time
Instruction observer, that "auto-removes" whenever it returns CONTINUE
or gets unblocked. Or just removes itself on first hit after it has done
its (suspended) task inspection.
But lets see how far we get with just providing the
TaskObserver.Instruction and see if that proves difficult for the
various use cases first. I suspect it will be easy to use in practise.
> Or maybe just add Action.STEP :D
There is value in providing just Action.CONTINUE and Action.BLOCK for
all observer interfaces. That way the code can be kept really generic.
And Action.STEP would be conceptually hard, since there can be multiple
observers that were triggered, so you end up with a set of 'actions' to
take. Then it might be hard to figure out what follows naturally after
that.
> > BTW, Sami/Adam preferred to see the observer called something like
> > Blocking rather than Instruction as that better reflected its behavior.
>
> Yup. We wanted to use it for blocking, we didnt have stepping in mind.
Renaming it to Blocking might also be confusing, since the Thread isn't
actually Blocked. There is just the potential to do a full Block on
every step. For now I kept the name TaskObserver.Instruction, but we can
choose another name of course.
I think that as soon as we are using it more and have some use cases it
will be easier to name. So please start using it :)
Cheers,
Mark
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Forward of blocked observer discussion
2006-10-12 19:24 ` Nurdin Premji
@ 2006-10-16 15:05 ` Mark Wielaard
0 siblings, 0 replies; 9+ messages in thread
From: Mark Wielaard @ 2006-10-16 15:05 UTC (permalink / raw)
To: Nurdin Premji; +Cc: frysk
Hi Nurdin,
On Thu, 2006-10-12 at 15:26 -0400, Nurdin Premji wrote:
> So what I want in terms of a blocked observer are two things.
> #1 A replacement for Mike's ProcAttachedObserver that looks fairly the
> same except all the work for existing tasks is called after the proc has
> been completely blocked (rather than in updateAttached as it currently
> is).
I was looking at this and suddenly noticed that when you send a SIGSTOP
to any of the Tasks in a Proc all Tasks stop, same for a SIGCONT. This
seems to solve our "block all threads" issue pretty nicely. We would
need a StoppedEvent observer though (see the handleStoppedEvent()
methods in TaskState, there is already some comments that we should
handle this).
This does however also mean that the way TaskState now handles temporary
suspension of a Task is a little bit more aggressive than I intended. I
don't know what the right solution is. We could use another signal and
more the handling code to handleSignaled().
It seems that this behavior is actually what Posix describes.
http://people.redhat.com/drepper/posix-signal-model.xml
I am not clear on whether modern GNU/Linux systems actually do this now.
Limited tests seem to work, but it seems gdb doesn't use it for some
reason. Isn't it reliable?
Cheers,
Mark
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2006-10-16 15:05 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-10-12 19:15 Forward of blocked observer discussion Nurdin Premji
2006-10-12 19:24 ` Nurdin Premji
2006-10-16 15:05 ` Mark Wielaard
2006-10-12 21:38 ` Andrew Cagney
2006-10-12 23:43 ` Mark Wielaard
2006-10-13 14:31 ` Andrew Cagney
2006-10-15 2:24 ` Mark Wielaard
2006-10-13 19:49 ` Sami Wagiaalla
2006-10-15 2:38 ` Mark Wielaard
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).