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 615078 - gi/function.c misuses g_callable_info_prepare_closure(), causing crashes
gi/function.c misuses g_callable_info_prepare_closure(), causing crashes
Status: RESOLVED FIXED
Product: gjs
Classification: Bindings
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gjs-maint
gjs-maint
Depends on:
Blocks:
 
 
Reported: 2010-04-07 16:25 UTC by Dan Winship
Modified: 2010-04-07 23:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
simple fix (3.33 KB, patch)
2010-04-07 17:17 UTC, Dan Winship
none Details | Review
Fix ffi_closure usage (4.08 KB, patch)
2010-04-07 23:34 UTC, Dan Winship
committed Details | Review

Description Dan Winship 2010-04-07 16:25:48 UTC
gi/function.c uses g_callable_info_prepare_closure() to allocate closures, but then frees them using munmap(), contrary to the g_callable_info_prepare_closure() docs which say you have to use g_callable_info_free_closure(). (Admittedly, it used to say that you had to use _prepare_closure to free the closure as well, but it was obvious what it *meant*.)

After the change to use ffi_closure_alloc() in _prepare_closure(), this now makes gjs crash after invoking a callback (except, for some reason, on x86_64).

Fixing this is a bit of a mess because invoke_info->closure is *sometimes* created by g_callable_info_prepare_closure, but sometimes created by hand, and so we'd need to free it differently in the two cases. (And the created-by-hand case for destroynotifies needs to be fixed to use ffi_closure_alloc() as well.)
Comment 1 Dan Winship 2010-04-07 17:17:25 UTC
Created attachment 158141 [details] [review]
simple fix
Comment 2 Dan Winship 2010-04-07 23:34:06 UTC
Created attachment 158152 [details] [review]
Fix ffi_closure usage

updated with a real commit message, and fixed to use ffi_prep_closure_loc()
as well.
Comment 3 Johan (not receiving bugmail) Dahlin 2010-04-07 23:37:50 UTC
Review of attachment 158152 [details] [review]:

Looks good.
Comment 4 Johan (not receiving bugmail) Dahlin 2010-04-07 23:37:51 UTC
Review of attachment 158152 [details] [review]:

Looks good.
Comment 5 Dan Winship 2010-04-07 23:47:40 UTC
Attachment 158152 [details] pushed as 87077da - Fix ffi_closure usage