Bug 135258 - JSWrapperMap's jsWrapperForObject() needs to keep weak prototype and constructors from being GCed
Summary: JSWrapperMap's jsWrapperForObject() needs to keep weak prototype and construc...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on: 135265
Blocks:
  Show dependency treegraph
 
Reported: 2014-07-24 14:34 PDT by Mark Lam
Modified: 2014-07-24 18:00 PDT (History)
8 users (show)

See Also:


Attachments
the patch. (3.49 KB, patch)
2014-07-24 14:42 PDT, Mark Lam
oliver: review+
Details | Formatted Diff | Diff
patch 2: new fix that does not defer GC (3.96 KB, patch)
2014-07-24 17:18 PDT, Mark Lam
mhahnenberg: 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 2014-07-24 14:34:54 PDT
In the process of creating a JS wrapper, jsWrapperForObject() will create the prototype and constructor of the corresponding ObjC class, as well as for classes in its inheritance chain.  These prototypes and constructors are stored in Weak references in the JSObjCClassInfo objects.  During all the allocation that is being done to create all the prototypes and constructors as well as the wrapper objects, a GC may occur thereby collecting one or more of these newly created prototype and constructor objects.

One example of where this problem can manifest is in wrapperForObject() which is called from jsWrapperForObject().  In wrapperFoObject(), we do the following steps:
1. reallocateConstructorAndOrPrototype() which creates the prototype object and store it in JSObjCClassInfo's m_prototype which is a Weak ref.
2. makeWrapper() to create the wrapper object, which may trigger a GC.  GC will collect the prototype object and nullify the corresponding JSObjCClassInfo's m_prototype Weak ref.
3. call JSObjectSetPrototype() to set the JSObjCClassInfo's m_prototype in the newly created wrapper.  This results in the wrapper getting a jsNull as a prototype instead of the expected prototype object.

To ensure that the prototype and constructor objects are retained until they can be referenced properly from the wrapper object, jsWrapperForObject() should defer GC until it's done with its work.
Comment 1 Mark Lam 2014-07-24 14:41:04 PDT
<rdar://problem/17757800>
Comment 2 Mark Lam 2014-07-24 14:42:31 PDT
Created attachment 235463 [details]
the patch.
Comment 3 Mark Lam 2014-07-24 15:03:58 PDT
Thanks.  Landed in r171527: <http://trac.webkit.org/r171527>.
Comment 4 WebKit Commit Bot 2014-07-24 16:11:31 PDT
Re-opened since this is blocked by bug 135265
Comment 5 Mark Lam 2014-07-24 17:18:44 PDT
Created attachment 235478 [details]
patch 2: new fix that does not defer GC
Comment 6 Mark Hahnenberg 2014-07-24 17:21:32 PDT
Comment on attachment 235478 [details]
patch 2: new fix that does not defer GC

r=me
Comment 7 Mark Lam 2014-07-24 18:00:57 PDT
Thanks.  New fix landed in r171564: <http://trac.webkit.org/r171564>.