Bug 195206 - [GLib] Returning G_TYPE_OBJECT from a constructor does not work
Summary: [GLib] Returning G_TYPE_OBJECT from a constructor does not work
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-03-01 03:25 PST by Adrian Perez
Modified: 2019-03-04 02:35 PST (History)
8 users (show)

See Also:


Attachments
jscobj.c: Example code to reproduce the issue (2.19 KB, text/x-csrc)
2019-03-01 03:25 PST, Adrian Perez
no flags Details
Patch (5.78 KB, patch)
2019-03-01 03:39 PST, Carlos Garcia Campos
zan: review+
Details | Formatted Diff | Diff
jscobjc-v2.c: Similar code, different issue (2.53 KB, text/x-csrc)
2019-03-01 07:59 PST, Adrian Perez
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Adrian Perez 2019-03-01 03:25:16 PST
Created attachment 363321 [details]
jscobj.c: Example code to reproduce the issue

When a constructor is installed with jsc_class_add_constructor(), and the
return type is specified as G_TYPE_OBJECT, methods do not seem to receive
the object returned from the constructor correctly when invoked.

For the small example that I am attaching, the output is:

  (process:9038): GLib-GIO-CRITICAL **: 12:39:24.868: g_file_get_path: assertion 'G_IS_FILE (file)' failed
  **
  ERROR:jscobj.c:54:main: assertion failed: (jsc_value_is_string (result))
  zsh: abort (core dumped)  ./jscobj
Comment 1 Carlos Garcia Campos 2019-03-01 03:36:36 PST
We are freeing the newly created object before returning from the constructor.
Comment 2 Carlos Garcia Campos 2019-03-01 03:39:01 PST
Created attachment 363322 [details]
Patch
Comment 3 Adrian Perez 2019-03-01 07:59:16 PST
Created attachment 363330 [details]
jscobjc-v2.c: Similar code, different issue

I have built JSC locally with your patch, and it does indeed
make it possible for the call to .getPath() to work correctly,
thanks!

Unfortunately, now there is a GObject critical during the
destruction of the JSCContext. I have am attaching an updated
test program; the output is:

  ** Message: 17:50:45.863: js_GFile_new: GFile @ 0x557a2fe3a220.
  ** Message: 17:50:45.863: js_GFile_getPath: GFile @ 0x557a2fe3a220.
  Path: /home/aperez

  (process:11638): GLib-GObject-CRITICAL **: 17:50:45.864: g_object_unref: assertion 'G_IS_OBJECT (object)' failed


Backtrace from GDB follows. I may be able to use a build of GLib
with debug info, so let me know if that would be helpful.


----- 8< ---- Backtrace ----- 8< ----------- 8< ----------- 8< -----

(gdb) r
Starting program: /home/aperez/jscobj 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/libthread_db.so.1".
[New Thread 0x7ffff4166700 (LWP 11916)]
[New Thread 0x7fffb2dff700 (LWP 11917)]
[New Thread 0x7fffb25fe700 (LWP 11918)]
** Message: 17:55:57.915: js_GFile_new: GFile @ 0x5555555a7000.
** Message: 17:55:57.916: js_GFile_getPath: GFile @ 0x5555555a7000.
Path: /home/aperez

(process:11912): GLib-GObject-CRITICAL **: 17:55:57.916: g_object_unref: assertion 'G_IS_OBJECT (object)' failed

Thread 1 "jscobj" received signal SIGTRAP, Trace/breakpoint trap.
0x00007ffff68ffc36 in ?? () from /usr/lib/libglib-2.0.so.0
(gdb) bt
#0  0x00007ffff68ffc36 in  () at /usr/lib/libglib-2.0.so.0
#1  0x00007ffff69005e3 in g_logv () at /usr/lib/libglib-2.0.so.0
#2  0x00007ffff69007e0 in g_log () at /usr/lib/libglib-2.0.so.0
#3  0x00007ffff79b15bf in void JSC::MarkedBlock::Handle::specializedSweep<false, (JSC::MarkedBlock::Handle::EmptyMode)0, (JSC::MarkedBlock::Handle::SweepMode)0, (JSC::MarkedBlock::Handle::SweepDestructionMode)0, (JSC::MarkedBlock::Handle::ScribbleMode)0, (JSC::MarkedBlock::Handle::NewlyAllocatedMode)0, (JSC::MarkedBlock::Handle::MarksMode)0, JSC::JSDestructibleObjectDestroyFunc>(JSC::FreeList*, JSC::MarkedBlock::Handle::EmptyMode, JSC::MarkedBlock::Handle::SweepMode, JSC::MarkedBlock::Handle::SweepDestructionMode, JSC::MarkedBlock::Handle::ScribbleMode, JSC::MarkedBlock::Handle::NewlyAllocatedMode, JSC::MarkedBlock::Handle::MarksMode, JSC::JSDestructibleObjectDestroyFunc const&) () at /home/aperez/.prefix/wkgtk-trunk/lib/libjavascriptcoregtk-4.0.so.18
#4  0x00007ffff79b1cb4 in void JSC::MarkedBlock::Handle::finishSweepKnowingHeapCellType<JSC::JSDestructibleObjectDestroyFunc>(JSC::FreeList*, JSC::JSDestructibleObjectDestroyFunc const&) () at /home/aperez/.prefix/wkgtk-trunk/lib/libjavascriptcoregtk-4.0.so.18
#5  0x00007ffff79abfe4 in JSC::JSDestructibleObjectHeapCellType::finishSweep(JSC::MarkedBlock::Handle&, JSC::FreeList*) () at /home/aperez/.prefix/wkgtk-trunk/lib/libjavascriptcoregtk-4.0.so.18
#6  0x00007ffff764e136 in JSC::MarkedBlock::Handle::sweep(JSC::FreeList*) () at /home/aperez/.prefix/wkgtk-trunk/lib/libjavascriptcoregtk-4.0.so.18
#7  0x00007ffff7620b4c in JSC::BlockDirectory::lastChanceToFinalize() () at /home/aperez/.prefix/wkgtk-trunk/lib/libjavascriptcoregtk-4.0.so.18
#8  0x00007ffff764a700 in JSC::MarkedSpace::lastChanceToFinalize() () at /home/aperez/.prefix/wkgtk-trunk/lib/libjavascriptcoregtk-4.0.so.18
#9  0x00007ffff7633887 in JSC::Heap::lastChanceToFinalize() () at /home/aperez/.prefix/wkgtk-trunk/lib/libjavascriptcoregtk-4.0.so.18
#10 0x00007ffff7b0b35b in JSC::VM::~VM() () at /home/aperez/.prefix/wkgtk-trunk/lib/libjavascriptcoregtk-4.0.so.18
#11 0x00007ffff79e67b8 in JSC::JSLockHolder::~JSLockHolder() () at /home/aperez/.prefix/wkgtk-trunk/lib/libjavascriptcoregtk-4.0.so.18
#12 0x00007ffff7033225 in JSContextGroupRelease () at /home/aperez/.prefix/wkgtk-trunk/lib/libjavascriptcoregtk-4.0.so.18
#13 0x00007ffff701e1ec in jscVirtualMachineSetContextGroup(_JSCVirtualMachine*, OpaqueJSContextGroup const*) [clone .part.58] () at /home/aperez/.prefix/wkgtk-trunk/lib/libjavascriptcoregtk-4.0.so.18
#14 0x00007ffff701e2ac in jscVirtualMachineDispose(_GObject*) () at /home/aperez/.prefix/wkgtk-trunk/lib/libjavascriptcoregtk-4.0.so.18
#15 0x00007ffff69f5315 in g_object_unref () at /usr/lib/libgobject-2.0.so.0
#16 0x00007ffff6ff3f32 in jscContextSetVirtualMachine(_JSCContext*, WTF::GRefPtr<_JSCVirtualMachine>&&) () at /home/aperez/.prefix/wkgtk-trunk/lib/libjavascriptcoregtk-4.0.so.18
#17 0x00007ffff6ff419b in jscContextDispose(_GObject*) () at /home/aperez/.prefix/wkgtk-trunk/lib/libjavascriptcoregtk-4.0.so.18
#18 0x00007ffff69f5315 in g_object_unref () at /usr/lib/libgobject-2.0.so.0
#19 0x00005555555552b0 in glib_autoptr_cleanup_JSCContext ()
#20 0x0000555555555595 in main ()
(gdb)
Comment 4 Carlos Garcia Campos 2019-03-01 08:15:47 PST
That's weird, do tests pass for you? because that case is now covered by the tests, I'm even checking the GFile is actually destroyed when the context is destroyed.
Comment 5 Carlos Garcia Campos 2019-03-01 09:05:05 PST
g_autoptr(JSCClass) klass = jsc_context_register_class 

That's the bug, you are freeing the JSCClass which is owned by the context.
Comment 6 Adrian Perez 2019-03-01 09:20:04 PST
(In reply to Carlos Garcia Campos from comment #5)
> g_autoptr(JSCClass) klass = jsc_context_register_class 
> 
> That's the bug, you are freeing the JSCClass which is owned by the context.

/me facepalms

You are right, and the API documentation definitely has the function
annotated “transfer none”. I removed the g_autoptr() and all works
like a charm now.

Informal r+ to the patch!
Comment 7 Carlos Garcia Campos 2019-03-04 02:34:11 PST
Committed r242349: <https://trac.webkit.org/changeset/242349>
Comment 8 Radar WebKit Bug Importer 2019-03-04 02:35:39 PST
<rdar://problem/48557414>