WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
144183
Simplify DOM wrapper destruction, don't deref() in finalizers.
https://bugs.webkit.org/show_bug.cgi?id=144183
Summary
Simplify DOM wrapper destruction, don't deref() in finalizers.
Andreas Kling
Reported
2015-04-24 21:13:05 PDT
Finalizers are not guaranteed to run for a Weak if its WeakImpl has been replaced by another (through use of Weak::operator=(Weak&&)) before the GC's incremental sweeper has swept the containing WeakBlock. Let's settle on a single way of invoking deref() on the DOM object.
Attachments
Proposed patch
(37.47 KB, patch)
2015-04-24 21:13 PDT
,
Andreas Kling
darin
: review+
Details
Formatted Diff
Diff
Patch for landing
(37.57 KB, patch)
2015-04-28 16:55 PDT
,
Andreas Kling
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Andreas Kling
Comment 1
2015-04-24 21:13:35 PDT
Created
attachment 251609
[details]
Proposed patch
Darin Adler
Comment 2
2015-04-26 11:52:39 PDT
Comment on
attachment 251609
[details]
Proposed patch View in context:
https://bugs.webkit.org/attachment.cgi?id=251609&action=review
> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1089 > + push(@headerContent, " void releaseImpl() { m_impl->deref(); m_impl = nullptr; }\n\n");
Could consider the suggestion Oliver Hunt made for the smart pointer classes: void releaseImpl() { std::exchange(m_impl, nullptr)->deref(); } That would mean that m_impl would be null if something happened to run in the destructor and turn around and see this object.
Andreas Kling
Comment 3
2015-04-28 16:55:57 PDT
Created
attachment 251899
[details]
Patch for landing With std::exchange like darin suggested.
Andreas Kling
Comment 4
2015-04-28 17:29:21 PDT
Comment on
attachment 251899
[details]
Patch for landing Holding cq+ while I check on a potential issue locally.
Andreas Kling
Comment 5
2015-04-28 18:25:45 PDT
Comment on
attachment 251899
[details]
Patch for landing Local issue was something else entirely. Resuming commit queue.
WebKit Commit Bot
Comment 6
2015-04-28 19:13:47 PDT
Comment on
attachment 251899
[details]
Patch for landing Clearing flags on attachment: 251899 Committed
r183523
: <
http://trac.webkit.org/changeset/183523
>
WebKit Commit Bot
Comment 7
2015-04-28 19:13:51 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug