Bug 146895 - Watchpoints should be removed from their owning WatchpointSet before they are fired
Summary: Watchpoints should be removed from their owning WatchpointSet before they are...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-07-12 14:34 PDT by Filip Pizlo
Modified: 2015-07-12 19:17 PDT (History)
13 users (show)

See Also:


Attachments
the patch (5.12 KB, patch)
2015-07-12 14:38 PDT, Filip Pizlo
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2015-07-12 14:34:44 PDT
This addresses two issues:

- We have had some weirdo bugs where the watchpoint being fired doesn't kill the watchpoint, and therefore doesn't remove it.  That causes an infinite loop inside WatchpointSet.  We don't have such bugs right now, but it's something that you can hit whenever adding new uses of the watchpoint API.  Having the removal happen before means that we just don't have to worry about this, ever.  OTOH, I can't think of any use of Watchpoint that wants to keep the Watchpoint in the set after it is fired.

- It would be great to write adaptive watchpoints that readd themselves to a different set when they are fired.  But to do this, we need to first remove them from the set that they are currently on.  The easiest API for this is to have the removal happen before fire() is called.
Comment 1 Filip Pizlo 2015-07-12 14:38:55 PDT
Created attachment 256682 [details]
the patch
Comment 2 WebKit Commit Bot 2015-07-12 14:42:06 PDT
Attachment 256682 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/bytecode/Watchpoint.h:83:  The parameter name "detail" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Filip Pizlo 2015-07-12 14:53:08 PDT
(In reply to comment #2)
> Attachment 256682 [details] did not pass style-queue:
> 
> 
> ERROR: Source/JavaScriptCore/bytecode/Watchpoint.h:83:  The parameter name
> "detail" adds no information, so it should be removed. 
> [readability/parameter_name] [5]
> Total errors found: 1 in 3 files
> 
> 
> If any of these errors are false positives, please file a bug against
> check-webkit-style.

Fixed locally.
Comment 4 Filip Pizlo 2015-07-12 19:17:45 PDT
Landed in http://trac.webkit.org/changeset/186745