[webkit-gtk] JavaScriptCore GObject Bindings - Memory Management

Fri Mar 17 07:54:47 PDT 2017

El vie, 17-03-2017 a las 22:58 +0900, Yusuke SUZUKI escribió:
> Thanks for clarifications!
> 
> On Fri, Mar 17, 2017 at 5:52 PM, Carlos Garcia Campos <cgarcia at igalia
> .com> wrote:
> > El mié, 15-03-2017 a las 14:54 +0900, Yusuke SUZUKI escribió:
> > > On Fri, Mar 10, 2017 at 9:49 PM, tevaum at gmail.com <tevaum at gmail.c
> > om>
> > > wrote:
> > > > Hi. I started implementing a JavaScriptCore GObject binding in
> > [1].
> > > > This API provides two main objects: JSCContext, representing
> > > > JSGlobalContextRef, and JSCValue, representing JSValueRef. It's
> > a
> > > > first patch and does't do any memory management, because I
> > wasn't
> > > > really aware of how JSC handles JSValueRefs and stuff.
> > > >
> > > > Asking on JSC list they replied that JSValues are garbage
> > collected
> > > > and when you need to keep a reference to it in the heap you
> > should
> > > > protect them. [2]
> > > >
> > >
> > > Yes. JSValues are GC-managed ones. So if it is on machine stack /
> > > registers, it will be marked and kept alive.
> > > And once it becomes unreachable, GC automatically collects these
> > > values.
> > 
> > Thanks for your help Yusuke, we really need help to understand how
> > things work in JSC to design our API.
> > 
> > > Note that JSValueRef is actually the value type rather than the
> > > pointer to something. (Internally, it something the pointer to
> > the
> > > JSString*, JSObject*.)
> > > It should be always copied. And it should not be allocated in
> > heap
> > > usually.
> > >  
> > > > This is already done by the bindings: JSValueRefs are protected
> > on
> > > > contruction of JSCValue and unprotected on destruction, but
> > > > JSCValues were implemented as initially owned and noone's
> > keeping
> > > > their reference to unref them when needed.
> > > >
> > >
> > > I think we should not protect/unprotect in API side
> > automatically.
> > > And I think we should not represent JSCValue as heap allocated
> > one.
> > > JSCValue should be represented like gint64 (value type).
> > 
> > What we really have is not a heap allocated JSValueRef, but a heap
> > allocated GObject wrapper for a JSValueRef. Similar to what JSValue
> > is
> > in the Objc API, a NSObject wrapping a JSValueRef. So, for example:
> > 
> > JSCValue *jsc_boolean_new (JSCContext *, gboolean);
> > 
> > JSCValue is the wrapper, and this simply calls JSValueMakeBoolean
> > to
> > create the JSVAlueRef wrapped by this GObject.
> 
> OK, so, in that case, JSCValueRef is stored in JSCValue. And JSCValue
> is allocated in the heap.
> Yeah, right. In that case, we should protect that value.
> I've just looked JSValue.mm & JSValue.h. Yup, they use
> protect/unprotect to offer such JSValue thing :) 
>  
> > In the Objc API values are always protected on construction and
> > unprotected on destruction. However, it also has API to create
> > managed
> > values, that I think they are like a weak ref.
> 
>  [JSValue valueWithJSValueRef:inContext:]?
> It seems that resulted JSValue is autorelease-ed. And they are held
> in weak map, thus we can internalize JSValues.

I was talking about JSManagedValue. Basically we want to avoid having
an API where the user has to unref or take care about ref counting in
most of the cases, but still allow the user to retain an object in some
cases. 

> > > Or if we would like to do that, I think we should separate it
> > from
> > > the usual JSValue class.
> > > They are responsibility of API users.
> > > Consider the following case,
> > >
> > > std::vector<JSValueRef> vector;
> > >
> > > In that case, the elements of the vector is allocated in the
> > heap.
> > > And GC does not know about this heap area.
> > > So GC does not mark objects in this heap area. In that case,
> > users
> > > should protect them.
> > 
> > What we have is something like this:
> > 
> > typedef struct _JSCValuePrivate {
> >     JSCContext* context;
> >     JSValueRef value;
> > } JSCValuePrivate;
> > 
> > And yes, that struct is always heap allocated.
> 
> OK, in that case, we should protect/unprotect it.
>  
> > > But if you use JSValueRef usually (not holding it in heap), we do
> > not
> > > need to protect it explicitly. GC will see your stack and
> > registers.
> > >
> > > I think the above use case is relatively rare: Usually, we create
> > the
> > > JSValue on the stack and pass it though the registers.
> > > When we would like to store these values in the heap, we should
> > store
> > > it in JSArray (js managed object) instead of users' vector or
> > > something.
> > >
> > > Protecting and unprotecting are just adding and removing the
> > value to
> > > the set of the GC root set.
> > 
> > So, I guess that if I create a JSValue that is heap allocated on a
> > context, and then I unprotect it, forcing a full GC would delete
> > the
> > JSValue immediately, right?
> 
> Is there any reference to the managed value in registers / stack(it
> includes x64 red zone)? (even such registers are not used)
> I guess GC marks it alive because of this.

Don't know, I'll try again.

> > I think I tried this for testing when we
> > started to work on this API and didn't work for me, that's why I'm
> > asking.
> > 
> > > It is not free, it adds and drops the cell to and from the hash
> > set.
> > > This is similar to V8's Persistent<>.
> > >  
> > > > So in the current state, we are leaking JSCValues and, as
> > JSCValues
> > > > protect JSValueRefs, memory is never freed.
> > > >
> > > > JavaScriptCore doesn't have any API to directly free
> > JSValueRefs,
> > > > so JSCValues needs to be destructed to unprotect JSValueRefs.
> > > >
> > > > My thought is to Adopt JSCValues when they are created using a
> > > > private API in the JSCContext
> > (jscContextAdoptJSCValue(JSCValue*)
> > > > ?). As JSCValues have a JSCContext reference, we can call
> > > > jscContextAdoptJSCValue in JSCValue's constructed callback.
> > With
> > > > this private API, JSCContext will get a JSCValue reference and
> > add
> > > > it to a HashMap that will map the JSValueRef to the JSCValue.
> > When
> > > > added to the HashMap, JSCValue will be owned by the context and
> > > > will be freed when the context is unrefed.
> > > >
> > > > What do you think about this idea?
> > > >
> > > > [1] https://bugs.webkit.org/show_bug.cgi?id=164061
> > > > [2] https://lists.webkit.org/pipermail/jsc-dev/2016-November/00
> > 0084
> > > > .html
> > > >
> > > > _______________________________________________
> > > > webkit-gtk mailing list
> > > > webkit-gtk at lists.webkit.org
> > > > https://lists.webkit.org/mailman/listinfo/webkit-gtk
> > > >
> > >
> > > _______________________________________________
> > > webkit-gtk mailing list
> > > webkit-gtk at lists.webkit.org
> > > https://lists.webkit.org/mailman/listinfo/webkit-gtk
> > --
> > Carlos Garcia Campos
> > http://pgp.rediris.es:11371/pks/lookup?op=get&search=0xF3D322D0EC45
> > 82C3
> 
> 
-- 
Carlos Garcia Campos
http://pgp.rediris.es:11371/pks/lookup?op=get&search=0xF3D322D0EC4582C3
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 224 bytes
Desc: This is a digitally signed message part
URL: <https://lists.webkit.org/pipermail/webkit-gtk/attachments/20170317/d8de66cb/attachment-0001.sig>