After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 779693 - gjs_eval_thread should always be set
gjs_eval_thread should always be set
Status: RESOLVED FIXED
Product: gjs
Classification: Bindings
Component: general
1.47.x
Other Linux
: Normal major
: ---
Assigned To: Philip Chimento
gjs-maint
Depends on:
Blocks:
 
 
Reported: 2017-03-07 03:37 UTC by Philip Chimento
Modified: 2017-03-08 02:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
context: Private method to get owner thread (3.69 KB, patch)
2017-03-07 23:21 UTC, Philip Chimento
committed Details | Review

Description Philip Chimento 2017-03-07 03:37:29 UTC
Unfortunately I can't reproduce this easily with any other objects from standard GNOME libraries. You can clone EosShard from [1]. It's pretty esoteric: it happens when a GBoxed (EosShardRecord in this case) unrefs a GObject (EosShardFile) during garbage collection.

The GObject is queued for idle toggle-down even though the unref happens on the main thread. This is because the check for gjs_eval_thread == g_thread_self() returns false. It turns out that gjs_eval_thread is never set in the first place, because it's set in the class_init function of the GJS custom object type, which is never called because we never create any custom objects.

Bizarrely, you can fix the crash by creating a dummy GObject.Class in this reproducer script:

    const GLib = imports.gi.GLib;
    const EosShard = imports.gi.EosShard;

    // Uncomment this to fix the crash
    // const GObject = imports.gi.GObject;
    // new GObject.Class({Name: 'Foo'});

    // This doesn't cause the crash but is needed for prepping the file
    let [fd, path] = GLib.file_open_tmp('fooXXXXXX.shard');
    let writer = new EosShard.WriterV2({ fd: fd });
    writer.add_record('0123456789abcdef');
    writer.finish();

    let shard = new EosShard.ShardFile({
        path: path,
    });
    shard.init(null);
    void shard.list_records();

This particular case is a regression from the GjsMaybeOwned changes I made for SpiderMonkey 38. The underlying bug may be older. I'm pretty sure there's another bug open about stuff being finalized on a background thread when it's not supposed to be, and this might be related. I can't find that bug, though.

[1] https://github.com/endlessm/eos-shard
Comment 1 Philip Chimento 2017-03-07 23:21:56 UTC
Created attachment 347435 [details] [review]
context: Private method to get owner thread

Each JSRuntime has an associated thread, an "owner thread" in
SpiderMonkey terminology. Most entry points into the JS engine will check
if the calling thread is the owner thread. Since we need to check if we
are on the owner thread in order to decide whether to handle toggle refs
on objects immediately or to defer them to an idle function, we record
the owner thread when constructing the GjsContext and provide an internal
method to retrieve it.

This makes it unnecessary to maintain a static pointer to the owner
thread in object.cpp, which was causing problems because it would not get
initialized if no custom classes were created.
Comment 2 Cosimo Cecchi 2017-03-08 00:25:34 UTC
Review of attachment 347435 [details] [review]:

Looks good to me.
Comment 3 Philip Chimento 2017-03-08 02:44:37 UTC
Attachment 347435 [details] pushed as c0fdda0 - context: Private method to get owner thread