Summary: | some Watchpoints' ::fireInternal method will call operations that might GC where the GC will cause the watchpoint itself to destruct | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Saam Barati <saam> | ||||||||||
Component: | JavaScriptCore | Assignee: | Saam Barati <saam> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | ap, benjamin, cdumez, cgarcia, commit-queue, fpizlo, ggaren, gskachkov, keith_miller, malvika.editsoftdigital, mark.lam, msaboff, oliver, sukolsak, webkit-bug-importer, ysuzuki | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Local Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Saam Barati
2016-06-28 00:40:11 PDT
Does this mean that WatchpointSet::fireBlah should have a DeferGC in it, so that individual watchpoints don't have to do it? (In reply to comment #2) > Does this mean that WatchpointSet::fireBlah should have a DeferGC in it, so > that individual watchpoints don't have to do it? I think that's how we should do it. This seems like the least error prone way to future proof new ::fireInternal implementations. Created attachment 282265 [details]
patch
Attachment 282265 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/bytecode/VariableWriteFireDetail.h:46: The parameter name "vm" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/WebCore/ChangeLog:9: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5]
ERROR: Source/JavaScriptCore/bytecode/Watchpoint.h:320: The parameter name "vm" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 3 in 27 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 282265 [details]
patch
I can dig it.
Created attachment 282267 [details]
patch for landing if bots are happy
Attachment 282267 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/bytecode/VariableWriteFireDetail.h:46: The parameter name "vm" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/bytecode/Watchpoint.h:320: The parameter name "vm" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 2 in 27 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 282269 [details]
lets see what the bots think
Attachment 282269 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/bytecode/Watchpoint.cpp:29: Alphabetical sorting problem. [build/include_order] [4]
Total errors found: 1 in 28 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 282272 [details]
lets see what the bots think
Comment on attachment 282272 [details] lets see what the bots think Clearing flags on attachment: 282272 Committed r202588: <http://trac.webkit.org/changeset/202588> All reviewed patches have been landed. Closing bug. |