Bug 198037

Summary:

[GLIB] Crash when instantiating a js object registered with jsc_context_register_class on window object cleared

Product:

WebKit

Reporter:

Carlos Garcia Campos <cgarcia>

Component:

JavaScriptCore

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

agomez, aperez, bugs-noreply, ews-watchlist, keith_miller, mark.lam, mcatanzaro, msaboff, saam, webkit-bug-importer

Priority:

Keywords:

Gtk, InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	mcatanzaro: review+

Description Carlos Garcia Campos 2019-05-20 05:31:36 PDT

This happens because JSCClass is keeping a pointer to the JSCContext used when the class is registered, and the context can be destroyed before the class.

Comment 1 Carlos Garcia Campos 2019-05-20 05:43:26 PDT

Created attachment 370249 [details]
Patch

Comment 2 Michael Catanzaro 2019-05-20 06:19:01 PDT

Comment on attachment 370249 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=370249&action=review

> Source/JavaScriptCore/API/glib/JSCClass.cpp:346
> -            static_cast<GParamFlags>(WEBKIT_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY)));
> +            static_cast<GParamFlags>(WEBKIT_PARAM_WRITABLE | G_PARAM_CONSTRUCT_ONLY)));

Nothing like a good API break to make a change exciting.

I think we can get away with this, though.

Comment 3 Carlos Garcia Campos 2019-05-20 06:48:11 PDT

Committed r245514: <https://trac.webkit.org/changeset/245514>

Comment 4 Radar WebKit Bug Importer 2019-05-20 06:49:19 PDT

<rdar://problem/50944708>

Comment 5 Adrian Perez 2019-05-20 07:54:52 PDT

(In reply to Michael Catanzaro from comment #2)
> Comment on attachment 370249 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=370249&action=review
> 
> > Source/JavaScriptCore/API/glib/JSCClass.cpp:346
> > -            static_cast<GParamFlags>(WEBKIT_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY)));
> > +            static_cast<GParamFlags>(WEBKIT_PARAM_WRITABLE | G_PARAM_CONSTRUCT_ONLY)));
> 
> Nothing like a good API break to make a change exciting.
> 
> I think we can get away with this, though.

Well, if people were using the API that allows registering JS
classes in the wild, we would have had a bug report for this
much earlier. So not many people are using this *for now*, and
I also think this change won't bite anybody ;-]