Bug 163990 - Add ThrowScope::release() calls at all call sites of jsMakeNontrivialString().
Summary: Add ThrowScope::release() calls at all call sites of jsMakeNontrivialString().
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-10-25 15:17 PDT by Mark Lam
Modified: 2016-10-25 15:57 PDT (History)
5 users (show)

See Also:


Attachments
proposed patch. (9.48 KB, patch)
2016-10-25 15:27 PDT, Mark Lam
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2016-10-25 15:17:04 PDT
Patch coming.
Comment 1 Mark Lam 2016-10-25 15:27:44 PDT
Created attachment 292834 [details]
proposed patch.
Comment 2 Geoffrey Garen 2016-10-25 15:29:36 PDT
Comment on attachment 292834 [details]
proposed patch.

r=me
Comment 3 Mark Lam 2016-10-25 15:33:39 PDT
Thanks for the review.  Landed in r207851: <http://trac.webkit.org/r207851>.
Comment 4 Darin Adler 2016-10-25 15:39:34 PDT
What’s the rule here? How can I be sure not to get this wrong in the future? Is the rule written down somewhere?
Comment 5 Mark Lam 2016-10-25 15:45:12 PDT
(In reply to comment #4)
> What’s the rule here? How can I be sure not to get this wrong in the future?
> Is the rule written down somewhere?

See the ChangeLog of r205569 for details on how the ExceptionScopes (ThrowScope and CatchScope) works: https://trac.webkit.org/changeset/205569/trunk/Source/JavaScriptCore/ChangeLog.

We're gradually converting the code base to do the right thing.  Once we've reach convergence, we'll turn on EXCEPTION_SCOPE_VERIFICATION on Debug builds.  That should help us catch places where we're not doing the right thing.
Comment 6 Darin Adler 2016-10-25 15:57:23 PDT
Lets put a copy of that somewhere else. We are going to have to follow these rules for years.