public inbox for frysk@sourceware.org
 help / color / mirror / Atom feed
* Adding and deleting observations multiple times
@ 2007-08-06 14:45 Mark Wielaard
  0 siblings, 0 replies; only message in thread
From: Mark Wielaard @ 2007-08-06 14:45 UTC (permalink / raw)
  To: frysk; +Cc: pmachata

[-- Attachment #1: Type: text/plain, Size: 954 bytes --]

Hi,

Petr found a bug with adding and observer multiple times and then
deleting it. He even wrote a testcase for it. The issue was that you
could add an observation for different events (or in his particular case
watch an breakpoint on multiple addresses) but that Proc doesn't know
about this and treats the double add and one delete as if the observer
should be deleted completely, so if you delete it for the another event
(address) the Proc sanity check kicks in and flags it as a double
delete. The following patch fixes that (and makes the test case slightly
more deterministic):

2007-08-06  Mark Wielaard  <mark@klomp.org>

    Fixes bug #4894
    * Proc.java (observations): Turn into a Collection that can
    contain an observer multiple times.
    * TestTaskObserverCode.java (testCodeRemovedInHit): Explicitly
    wait for add and delete.

With this patch the test case now succeeds and no regressions (tested on
x86_64 FC6).

Cheers,

Mark

[-- Attachment #2: add-delete-observation.patch --]
[-- Type: text/x-patch, Size: 2187 bytes --]

Index: frysk-core/frysk/proc/Proc.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/Proc.java,v
retrieving revision 1.112
diff -u -r1.112 Proc.java
--- frysk-core/frysk/proc/Proc.java	2 Jul 2007 14:40:17 -0000	1.112
+++ frysk-core/frysk/proc/Proc.java	6 Aug 2007 14:30:02 -0000
@@ -41,6 +41,7 @@
 package frysk.proc;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -472,9 +473,11 @@
   }
 
   /**
-   * The set of observations that currently apply to this task.
+   * The set of observations that currently apply to this task.  Note
+   * that this is a Collection that may contain the same Observer
+   * object multiple times (for possible different observations).
    */
-  private Set observations = new HashSet();
+  private Collection observations = new LinkedList();
 
   public boolean addObservation (Object o)
   {
Index: frysk-core/frysk/proc/TestTaskObserverCode.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/TestTaskObserverCode.java,v
retrieving revision 1.7
diff -u -r1.7 TestTaskObserverCode.java
--- frysk-core/frysk/proc/TestTaskObserverCode.java	3 Aug 2007 23:33:33 -0000	1.7
+++ frysk-core/frysk/proc/TestTaskObserverCode.java	6 Aug 2007 14:30:02 -0000
@@ -161,15 +161,16 @@
     RemovingCodeObserver code = new RemovingCodeObserver();
     task.requestUnblock(attachedObserver);
     task.requestAddCodeObserver(code, address1);
+    assertRunUntilStop("add breakpoint observer at address1");
     task.requestAddCodeObserver(code, address2);
-    assertRunUntilStop("add breakpoint observer");
+    assertRunUntilStop("add breakpoint observer at address1");
 
     assertEquals(code.hits, 0);
 
     // Request a run and watch the breakpoint get hit.
     requestDummyRun();
-    while (!terminatingObserver.terminating)
-      assertRunUntilStop("task terminating");
+    assertRunUntilStop("wait for delete 1");
+    assertRunUntilStop("wait for delete 2");
 
     assertEquals(code.hits, 2);
     assertEquals(code.deletes, 2);

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

only message in thread, other threads:[~2007-08-06 14:45 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-08-06 14:45 Adding and deleting observations multiple times 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).