Bug 148280

Summary: Unify code paths for manually deleting all code
Product: WebKit Reporter: Geoffrey Garen <ggaren>
Component: New BugsAssignee: Geoffrey Garen <ggaren>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, rniwa
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 148347    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews106 for mac-mavericks-wk2
none
Archive of layout-test-results from ews102 for mac-mavericks
none
Patch saam: review+

Geoffrey Garen
Reported 2015-08-20 18:44:22 PDT
Unify code paths for manually deleting all code
Attachments
Patch (10.29 KB, patch)
2015-08-20 18:53 PDT, Geoffrey Garen 		no flags
DetailsFormatted DiffDiff
Archive of layout-test-results from ews106 for mac-mavericks-wk2 (583.56 KB, application/zip)
2015-08-20 19:46 PDT, Build Bot 		no flags
Details
Archive of layout-test-results from ews102 for mac-mavericks (599.99 KB, application/zip)
2015-08-20 19:56 PDT, Build Bot 		no flags
Details
Patch (14.35 KB, patch)
2015-08-21 15:20 PDT, Geoffrey Garen 		saam: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Geoffrey Garen
Comment 1 2015-08-20 18:53:30 PDT
Created attachment 259557 [details] Patch
WebKit Commit Bot
Comment 2 2015-08-20 18:55:51 PDT
Attachment 259557 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/VMEntryScope.cpp:60: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/JavaScriptCore/runtime/VMEntryScope.h:46: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/JavaScriptCore/runtime/VMEntryScope.h:51: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 3 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Build Bot
Comment 3 2015-08-20 19:46:18 PDT
Comment on attachment 259557 [details] Patch Attachment 259557 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/83879 New failing tests: inspector/dom-debugger/node-removed.html
Build Bot
Comment 4 2015-08-20 19:46:20 PDT
Created attachment 259569 [details] Archive of layout-test-results from ews106 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Build Bot
Comment 5 2015-08-20 19:56:44 PDT
Comment on attachment 259557 [details] Patch Attachment 259557 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/83920 New failing tests: inspector/dom-debugger/node-removed.html
Build Bot
Comment 6 2015-08-20 19:56:46 PDT
Created attachment 259570 [details] Archive of layout-test-results from ews102 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-mavericks Platform: Mac OS X 10.9.5
Saam Barati
Comment 7 2015-08-21 00:46:26 PDT
Comment on attachment 259557 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259557&action=review > Source/JavaScriptCore/inspector/agents/InspectorRuntimeAgent.cpp:340 > + vm.deleteAllCode(); I don't think this is correct. If we don't immediately delete all code, the type profiling code has the chance of incurring a use after free bug because the above line may deallocate all type profiler related data structures. The executing JS code may refer to these data structures and modify them.
Geoffrey Garen
Comment 8 2015-08-21 15:20:05 PDT
Created attachment 259669 [details] Patch
WebKit Commit Bot
Comment 9 2015-08-21 15:21:24 PDT
Attachment 259669 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/VMEntryScope.cpp:60: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/JavaScriptCore/runtime/VMEntryScope.h:46: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/JavaScriptCore/runtime/VMEntryScope.h:51: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 3 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Saam Barati
Comment 10 2015-08-21 15:28:46 PDT
Comment on attachment 259669 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259669&action=review r=me > Source/JavaScriptCore/runtime/VM.cpp:475 > +void VM::afterVMExit(std::function<void()> callback) I wonder if there is a better name for this. Just reading this name without reading its implementation indicates to me that this is always called asynchronously.
Geoffrey Garen
Comment 11 2015-08-21 15:46:47 PDT
> > Source/JavaScriptCore/runtime/VM.cpp:475 > > +void VM::afterVMExit(std::function<void()> callback) > > I wonder if there is a better name for this. Just reading this name without > reading its implementation > indicates to me that this is always called asynchronously. Switch when whenIdle.
Geoffrey Garen
Comment 12 2015-08-21 15:48:57 PDT
Committed r188792: <http://trac.webkit.org/changeset/188792>
WebKit Commit Bot
Comment 13 2015-08-21 17:49:28 PDT
Re-opened since this is blocked by bug 148347
Geoffrey Garen
Comment 14 2015-08-23 19:08:22 PDT
Committed r188846: <http://trac.webkit.org/changeset/188846>
Note You need to log in before you can comment on or make changes to this bug.