Bug 148280 - Unify code paths for manually deleting all code
Summary: Unify code paths for manually deleting all code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Geoffrey Garen
URL:
Keywords:
Depends on: 148347
Blocks:
  Show dependency treegraph
 
Reported: 2015-08-20 18:44 PDT by Geoffrey Garen
Modified: 2015-08-23 19:08 PDT (History)
3 users (show)

See Also:


Attachments
Patch (10.29 KB, patch)
2015-08-20 18:53 PDT, Geoffrey Garen
no flags Details | Formatted Diff | Diff
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+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2015-08-20 18:44:22 PDT
Unify code paths for manually deleting all code
Comment 1 Geoffrey Garen 2015-08-20 18:53:30 PDT
Created attachment 259557 [details]
Patch
Comment 2 WebKit Commit Bot 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.
Comment 3 Build Bot 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
Comment 4 Build Bot 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
Comment 5 Build Bot 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
Comment 6 Build Bot 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
Comment 7 Saam Barati 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.
Comment 8 Geoffrey Garen 2015-08-21 15:20:05 PDT
Created attachment 259669 [details]
Patch
Comment 9 WebKit Commit Bot 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.
Comment 10 Saam Barati 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.
Comment 11 Geoffrey Garen 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.
Comment 12 Geoffrey Garen 2015-08-21 15:48:57 PDT
Committed r188792: <http://trac.webkit.org/changeset/188792>
Comment 13 WebKit Commit Bot 2015-08-21 17:49:28 PDT
Re-opened since this is blocked by bug 148347
Comment 14 Geoffrey Garen 2015-08-23 19:08:22 PDT
Committed r188846: <http://trac.webkit.org/changeset/188846>