[Webkit-unassigned] [Bug 70015] Constructor should not be called if the object is being constructed inside WebCore

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Oct 13 17:48:27 PDT 2011


https://bugs.webkit.org/show_bug.cgi?id=70015





--- Comment #5 from Kentaro Hara <haraken at chromium.org>  2011-10-13 17:48:27 PST ---
(In reply to comment #2)
> (From update of attachment 110823 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=110823&action=review
> 
> > Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:1536
> > +    if (AllowAllocation::current())
> > +        return args.Holder();
> 
> Do we need this check in any of our custom constructors as well?  I don't fully understand in which cases AllowAllocation::current() return true.

Adam: AllowAllocation::current() just calls Isolate::GetCurrent(), and Isolate::GetCurrent() returns the entered isolate for the current thread or NULL in case there is no current isolate (See the comment of Isolate::GetCurrent(): http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/v8/include/v8.h&type=cs&l=2727). In my understanding, this means that AllowAllocation::current() returns true if it is called inside toV8() which is invoked from the WebCore context, which is the case where constructorCallback() should not be executed.

This patch added the AllowAllocation check to all non-custom constructors of V8. I think that the AllowAllocation check should be also in all custom constructors of V8. Indeed, as for existing custom constructors, no bugs have been occurring without the AllowAllocation check. However, (1) the Problem1 and Problem2 that I described above can happen in the future if someone changes code, and (2) if the AllowAllocation check does not exist in the existing custom constructors, people will add a new custom constructor without the AllowAllocation check without considering the possibility of Problem1 and Problem2, which may result in ugly bugs. 

WDYT?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list