Skip to content

StyleContext.lookupColor causes GSlice MemChecker to fail #221

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
gnunn1 opened this issue Nov 30, 2017 · 11 comments
Closed

StyleContext.lookupColor causes GSlice MemChecker to fail #221

gnunn1 opened this issue Nov 30, 2017 · 11 comments

Comments

@gnunn1
Copy link
Contributor

gnunn1 commented Nov 30, 2017

I'm working on resolving some crash issues in Tilix (gnunn1/tilix#1192). Running tilix with GSLICE=debug_blocks shows a problem with the StyleContext.lookupColor method that returns an RGBA, attempt to release non-allocated block. The full stack trace is shown below.

Looking at the code for lookupColor, I can see that memory is being allocated for the RGBA so GtkD should own it, I'm not seeing why it has already been released? Note that most of the RGBA instances I am using are throwaway in the sense they can used in a method and go out of scope as soon as the method completes. The error occurs when the D GC is invoked.

If you want to reproduce it with tilix:

dub build --build=debug --config=trace
G_SLICE=debug-blocks gdb ./tilix
run

Once tilix is running, open the preference dialog and close it. If it doesn't happen, go into the hamburger menu and select GC to force a garbage collection cycle.

Here is the stack trace.

GSlice: MemChecker: attempt to release non-allocated block: 0x555557076510 size=32

Thread 1 "tilix" received signal SIGABRT, Aborted.
0x00007ffff64078a0 in raise () from /usr/lib/libc.so.6
(gdb) bt
#0 0x00007ffff64078a0 in raise () at /usr/lib/libc.so.6
#1 0x00007ffff6408f09 in abort () at /usr/lib/libc.so.6
#2 0x00007ffff3cb0e09 in g_slice_free1 () at /usr/lib/libglib-2.0.so.0
#3 0x0000555555e49c8e in _D3gdk4RGBAQf6__dtorMFZv (this=0x7ffff7f390c0) at ../../../.dub/packages/gtk-d-3.7.2/gtk-d/generated/gtkd/gdk/RGBA.d:72
#4 0x00007ffff7510a87 in rt_finalize2 () at /usr/lib/libphobos2.so.0.77
#5 0x00007ffff7510b8a in rt_finalizeFromGC () at /usr/lib/libphobos2.so.0.77
#6 0x00007ffff75020c2 in _D2gc4impl12conservativeQw3Gcx5sweepMFNbZm () at /usr/lib/libphobos2.so.0.77
#7 0x00007ffff7502a11 in _D2gc4impl12conservativeQw3Gcx11fullcollectMFNbbZm () at /usr/lib/libphobos2.so.0.77
#8 0x00007ffff75007f2 in _D2gc4impl12conservativeQw3Gcx10smallAllocMFNbhKmkZPv () at /usr/lib/libphobos2.so.0.77
#9 0x00007ffff7504885 in _D2gc4impl12conservativeQw14ConservativeGC__T9runLockedS_DQCeQCeQCcQCnQBs12mallocNoSyncMFNbmkKmxC8TypeInfoZPvS_DQEgQEgQEeQEp10mallocTimelS_DQFiQFiQFgQFr10numMallocslTmTkTmTxQCzZQFcMFNbKmKkKmKxQDsZQDl () at /usr/lib/libphobos2.so.0.77
#10 0x00007ffff74fe36b in D2gc4impl12conservativeQw14ConservativeGC6qallocMFNbmkxC8TypeInfoZS4core6memory8BlkInfo () at /usr/lib/libphobos2.so.0.77
#11 0x00007ffff74fd59b in gc_qalloc () at /usr/lib/libphobos2.so.0.77
#12 0x00007ffff74f0470 in D4core6memory2GC6qallocFNaNbmkxC8TypeInfoZSQBqQBo8BlkInfo () at /usr/lib/libphobos2.so.0.77
#13 0x00007ffff739c5c4 in _D3std5array__T8AppenderTAyaZQo13ensureAddableMFNaNbNemZv () at /usr/lib/libphobos2.so.0.77
#14 0x0000555555cebaf7 in _D3std5array__T8AppenderTAyaZQo__T3putTQoZQiMFQvZ10bigDataFunMFNaNbNemZAa (this=0x7fffffffcc80, extra=4)
at /usr/include/dlang/dmd/std/array.d:3043
#15 0x0000555555ceba39 in _D3std5array__T8AppenderTAyaZQo__T3putTQoZQiMFNaNbNfQBbZv (this=..., items=...) at /usr/include/dlang/dmd/std/array.d:3046
#16 0x00007ffff744cffd in _D3std3xml__T6encodeTAyaZQmFNaNbNfQnZQq () at /usr/lib/libphobos2.so.0.77
#17 0x00007ffff7446b78 in _D3std3xml4Text6__ctorMFNaNfAyaZCQBfQBeQBd () at /usr/lib/libphobos2.so.0.77
#18 0x00007ffff7447951 in std.xml.ElementParser.parse() () at /usr/lib/libphobos2.so.0.77
#19 0x0000555555de38f9 in gx.tilix.prefeditor.prefdialog.ShortcutPreferences.loadLocalizedShortcutLabels() (this=0x7ffff7f44800)
at source/gx/tilix/prefeditor/prefdialog.d:926
#20 0x0000555555de415f in _D2gx5tilix10prefeditor10prefdialog19ShortcutPreferences13loadShortcutsMFC3gtk9TreeStoreQkZv (this=0x7ffff7f44800, ts=0x7ffff7f3e100)
at source/gx/tilix/prefeditor/prefdialog.d:958
#21 0x0000555555de233f in gx.tilix.prefeditor.prefdialog.ShortcutPreferences.createUI() (this=0x7ffff7f44800) at source/gx/tilix/prefeditor/prefdialog.d:734
#22 0x0000555555de46e9 in _D2gx5tilix10prefeditor10prefdialog19ShortcutPreferences6__ctorMFC3gio8SettingsQjZCQDdQDdQDaQCrQCi (this=0x7ffff7f44800, gsSettings=0x7ffff7efbf00) at source/gx/tilix/prefeditor/prefdialog.d:1002
#23 0x0000555555dded3c in gx.tilix.prefeditor.prefdialog.PreferenceDialog.createUI() (this=0x7ffff7f02000) at source/gx/tilix/prefeditor/prefdialog.d:153
#24 0x0000555555de096b in _D2gx5tilix10prefeditor10prefdialog16PreferenceDialog6__ctorMFC3gtk17ApplicationWindowQtZCQDkQDkQDhQCyQCp (this=0x7ffff7f02000, window=0x7fffe938d000) at source/gx/tilix/prefeditor/prefdialog.d:412
#25 0x0000555555d945ec in gx.tilix.application.Tilix.presentPreferences() (this=0x7ffff7edc800) at source/gx/tilix/application.d:760
#26 0x0000555555d9155a in gx.tilix.application.Tilix.onShowPreferences() (this=0x7ffff7edc800) at source/gx/tilix/application.d:274
#27 0x0000555555d91344 in _D2gx5tilix11application5Tilix14installAppMenuMFZ__T12__dgliteral5TC4glib7VariantQiTC3gio12SimpleActionQoZQCcMFQBsQBeZv (this=0x7ffff7eb4440, SimpleAction=0x7ffff7eb9200, GVariant=0x7ffff7efef20) at source/gx/tilix/application.d:210
#28 0x0000555555e73b4a in _D3gio12SimpleActionQo16callBackActivateUPSQBp1c5types13GSimpleActionPS4glibQBeQBf8GVariantCQDmQDlQDo25OnActivateDelegateWrapperZv (simpleactionStruct=0x5555566868a0, parameter=0x0, wrapper=0x7ffff7eb5d00) at ../../../.dub/packages/gtk-d-3.7.2/gtk-d/generated/gtkd/gio/SimpleAction.d:267
#29 0x00007ffff26516f5 in g_closure_invoke () at /usr/lib/libgobject-2.0.so.0
#30 0x00007ffff26650b0 in () at /usr/lib/libgobject-2.0.so.0
@MikeWey
Copy link
Member

MikeWey commented Nov 30, 2017

The problem seems to be that GtkD uses g_malloc to allocate the GdkRGBA struct, while gdk_rgba_free uses the g_slice API.

We should probably try switching to the g_slice API as it's recommended over malloc.

MikeWey added a commit that referenced this issue Nov 30, 2017
@MikeWey
Switch to using the Slice allocator to allocate structs.
9803474 
Most of the time the time the C side uses the slice allocator, and the
free functions will in that case call g_slice_free and that fails if the
memory wasn't used to allocate the memory.

See Also: #221
@MikeWey
Copy link
Member

MikeWey commented Nov 30, 2017

I can't reproduce the crash when using malloc but the GSlice: MemChecker: error is fixed when using sliceAlloc.

@gnunn1
Copy link
Contributor Author

gnunn1 commented Dec 1, 2017

Using your updated code I no longer have the GSlice: MemChecker issue however I've noticed a new problem with this change. I have a patched VTE that when detected calls some additional code which in turn calls the Terminal.getTextRange method. With your change to gslice it now fails with a segfault, however with the old gmalloc call it works fine.

Here is where I call the method:

https://github.com/gnunn1/tilix/blob/master/source/gx/tilix/terminal/terminal.d#L1490

BTW, I realize I don't need to create the attributes object and I'll fix that however it makes no difference for this issue.

I have pasted the stack trace below.

2017-11-30T19:03:11.328:appwindow.d:__dgliteral8:1726 Window state changed

Thread 1 "tilix" received signal SIGSEGV, Segmentation fault.
0x00007ffff652ae3d in __memset_avx2_erms () from /usr/lib/libc.so.6
(gdb) bt
#0  0x00007ffff652ae3d in __memset_avx2_erms () at /usr/lib/libc.so.6
#1  0x00007ffff3cdc6bc in g_array_set_size () at /usr/lib/libglib-2.0.so.0
#2  0x00007fffed9abf3a in  () at /usr/lib/libvte-2.91.so.0
#3  0x00007fffed9c1d34 in vte_terminal_get_text_range () at /usr/lib/libvte-2.91.so.0
#4  0x0000555555e2be09 in _D3vte8TerminalQj12getTextRangeMFllllPUPSQBn1c5types11VteTerminalllPvZiQeJC4glib6ArrayGQhZAya (this=0x7fffe938f000, attributes=@0x7fffffffd450: 0x0, userData=0x0, isSelected=0x0, endCol=0, endRow=0, startCol=-1, startRow=0) at ../GtkD/generated/vte/vte/Terminal.d:586
#5  0x0000555555e0b75f in _D2gx5tilix8terminalQj8Terminal18onVTECheckTriggersMFC3vteQBkQBnZv (this=0x7fffe938e000, _param_0=0x7fffe938f000)
    at source/gx/tilix/terminal/terminal.d:1491
#6  0x0000555555e2de2d in _D3vte8TerminalQj23callBackContentsChangedUPSQBr1c5types11VteTerminalCQCqQCpQCs32OnContentsChangedDelegateWrapperZv (terminalStruct=0x5555566926a0, wrapper=0x7ffff7edd980) at ../GtkD/generated/vte/vte/Terminal.d:1830
#7  0x00007ffff26516f5 in g_closure_invoke () at /usr/lib/libgobject-2.0.so.0
#8  0x00007ffff2664eff in  () at /usr/lib/libgobject-2.0.so.0
#9  0x00007ffff2669696 in g_signal_emit_valist () at /usr/lib/libgobject-2.0.so.0
#10 0x00007ffff266a920 in g_signal_emit () at /usr/lib/libgobject-2.0.so.0
#11 0x00007fffed9b5eac in  () at /usr/lib/libvte-2.91.so.0
#12 0x00007fffed9b77a8 in  () at /usr/lib/libvte-2.91.so.0
#13 0x00007fffed9b7a04 in  () at /usr/lib/libvte-2.91.so.0
#14 0x00007ffff3cbdcb3 in  () at /usr/lib/libglib-2.0.so.0
#15 0x00007ffff3cbf0be in g_main_context_dispatch () at /usr/lib/libglib-2.0.so.0
#16 0x00007ffff3cc0f69 in  () at /usr/lib/libglib-2.0.so.0
#17 0x00007ffff3cc0fae in g_main_context_iteration () at /usr/lib/libglib-2.0.so.0
#18 0x00007ffff28d25ae in g_application_run () at /usr/lib/libgio-2.0.so.0
#19 0x0000555555e5f49e in _D3gio11ApplicationQn3runMFAAyaZi (this=0x7ffff7edc800, argv=...) at ../GtkD/generated/gtkd/gio/Application.d:931
#20 0x0000555555ce42fd in D main (args=...) at source/app.d:146

@MikeWey
Copy link
Member

MikeWey commented Dec 1, 2017

GArray is an interesting case, as it uses a different type internally to represent the array. So the size of the slice allocated by GtkD doesn't match the size GArray tries to free.

@gnunn1
Copy link
Contributor Author

gnunn1 commented Dec 1, 2017

That's interesting, I assume the appropriate allocator to be used can be detected by the generator based on the return type?

@MikeWey
Copy link
Member

MikeWey commented Dec 2, 2017

Not as far as i can tell, using malloc we would have the same bug only without the segfault which isn't a good option.

GtkD never frees the GArray (which should be looked into) which is why there is no error when using malloc, the segfault is triggered when getTextRange tries to read/write past the allocated memory.

We either need to add some padding bytes to the struct definitions in GtkD, or special case the GArray types in the generator to use _new.

MikeWey added a commit that referenced this issue Dec 2, 2017
@MikeWey
Allocate the GLib array types with their new functions.
8fc625d 
See Also: #221
@MikeWey
Copy link
Member

MikeWey commented Dec 2, 2017

I've changed sliceNew to use the _new functions to allocate the GLib array types.

@gnunn1
Copy link
Contributor Author

gnunn1 commented Dec 3, 2017

I built a binary based on GtkD master for my user that has been having crash and he's reported no crashes so far so thanks for your help. I would give it a few days to be sure but would a point release of GtkD be possible if it looks good?

@MikeWey
Copy link
Member

MikeWey commented Dec 3, 2017

Great, a point release would be in order if this fixes the crash.

@gnunn1
Copy link
Contributor Author

gnunn1 commented Dec 16, 2017

@MikeWey It looks like this fixes the issue, my original issue reporter is finding tilix is stable now and it seems good for me as well. If you could push out a new GtkD point release when you get a chance I'd appreciate it.

@MikeWey
Copy link
Member

MikeWey commented Dec 16, 2017

Done.

@MikeWey MikeWey closed this as completed Dec 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@MikeWey @gnunn1