Bug 167193 - JSSegmentedVariableObject and its subclasses should have a sane destruction story
Summary: JSSegmentedVariableObject and its subclasses should have a sane destruction s...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-18 20:21 PST by Filip Pizlo
Modified: 2017-01-18 21:14 PST (History)
1 user (show)

See Also:


Attachments
the patch (45.87 KB, patch)
2017-01-18 20:43 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (45.86 KB, patch)
2017-01-18 20:52 PST, Filip Pizlo
sbarati: 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 2017-01-18 20:21:42 PST
Right now, JSSegmentedVariableObjects' subclasses install finalizers that call destroy. They do this in random ways, which sometimes result in JSSegmentedVariableObject::~JSSegmentedVariableObject executing more than once (which works because of the way that ~SegmentedVector is written). Maybe this works now, but it's a disaster waiting to happen.

Fortunately we can now just give those things their own Subspace and teach it its own protocol of destruction.
Comment 1 Filip Pizlo 2017-01-18 20:43:24 PST
Created attachment 299227 [details]
the patch
Comment 2 WebKit Commit Bot 2017-01-18 20:44:29 PST
Attachment 299227 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/JSSegmentedVariableObject.h:104:  The parameter name "vm" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/JSSegmentedVariableObjectSubspace.cpp:29:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Filip Pizlo 2017-01-18 20:52:39 PST
Created attachment 299230 [details]
the patch
Comment 4 Filip Pizlo 2017-01-18 21:14:55 PST
Landed in https://trac.webkit.org/changeset/210912