yabar icon indicating copy to clipboard operation
yabar copied to clipboard

coredump: ya_draw_pango_text

Open mwberry opened this issue 4 years ago • 5 comments

I've been using yabar for a few days and I like it a lot, however it quite frequently crashes on my system. I've installed it via the AUR package, but I removed -O2 and added -g for the sake of getting core dumps. So far I've seen it crash about 7 times in the last week-ish. The last two times I had debug info and the crashes came from ya_int_date and ya_int_memory, but I suspect all calls to ya_draw_pango_text are susceptible.

Originally I thought it might have had to do with waking up from suspend, but I've now seen it occur while the machine was idle running xscreensaver.

I'll see how much effort it will be to compile cairo with debug information, to get a better understanding of what invariant is being violated.

                #0  0x00007f9777f4bce5 raise (libc.so.6 + 0x3bce5)
                #1  0x00007f9777f35857 abort (libc.so.6 + 0x25857)
                #2  0x00007f9777f8f2b0 __libc_message (libc.so.6 + 0x7f2b0)
                #3  0x00007f9777f9674a malloc_printerr (libc.so.6 + 0x8674a)
                #4  0x00007f9777f99a44 _int_malloc (libc.so.6 + 0x89a44)
                #5  0x00007f9777f9afb9 malloc (libc.so.6 + 0x8afb9)
                #6  0x00007f9778418523 n/a (libcairo.so.2 + 0xaa523)
                #7  0x00007f9778418afa cairo_xcb_surface_create (libcairo.so.2 + 0xaaafa)
                #8  0x000055acace55156 ya_draw_pango_text (yabar + 0x8156)
                #9  0x000055acace5b67c ya_int_memory (yabar + 0xe67c)
                #10 0x000055acace56732 ya_exec (yabar + 0x9732)
                #11 0x00007f9778bd446f start_thread (libpthread.so.0 + 0x946f)
                #12 0x00007f977800f3d3 __clone (libc.so.6 + 0xff3d3)
                #0  0x00007f519d8d6ce5 raise (libc.so.6 + 0x3bce5)
                #1  0x00007f519d8c0857 abort (libc.so.6 + 0x25857)
                #2  0x00007f519d91a2b0 __libc_message (libc.so.6 + 0x7f2b0)
                #3  0x00007f519d92174a malloc_printerr (libc.so.6 + 0x8674a)
                #4  0x00007f519d924a44 _int_malloc (libc.so.6 + 0x89a44)
                #5  0x00007f519d925fb9 malloc (libc.so.6 + 0x8afb9)
                #6  0x00007f519dda3523 n/a (libcairo.so.2 + 0xaa523)
                #7  0x00007f519dda3afa cairo_xcb_surface_create (libcairo.so.2 + 0xaaafa)
                #8  0x0000560c9d628156 ya_draw_pango_text (yabar + 0x8156)
                #9  0x0000560c9d62d7da ya_int_date (yabar + 0xd7da)
                #10 0x0000560c9d629732 ya_exec (yabar + 0x9732)
                #11 0x00007f519e55f46f start_thread (libpthread.so.0 + 0x946f)
                #12 0x00007f519d99a3d3 __clone (libc.so.6 + 0xff3d3)

mwberry avatar Mar 24 '20 04:03 mwberry

stderr during a crash:

$ yabar
INH ya_bat
E: memory
E: cpu
E: workspaces
E: network
E: ya_bat
INH ya_bat
E: space
E: space
E: title
E: ya_bat
INH ya_bat
E: cpu
E: memory
E: network
E: date
malloc(): mismatching next->prev_size (unsorted)
zsh: abort (core dumped)  yabar

mwberry avatar Mar 28 '20 19:03 mwberry

So far, running under valgrind memcheck has not revealed where the memory corruption is occurring.

However, running under helgrind pretty quickly honed on some potential data races around ya_draw_pango_text.

==326615== ----------------------------------------------------------------
==326615== 
==326615==  Lock at 0x5F07B38 was first observed
==326615==    at 0x4841AF9: pthread_mutex_init (hg_intercepts.c:787)
==326615==    by 0x113C23: ya_setup_bar (ya_parse.c:374)
==326615==    by 0x114E4E: ya_config_parse (ya_parse.c:771)
==326615==    by 0x111F39: ya_init (ya_exec.c:430)
==326615==    by 0x1128EC: main (ya_main.c:16)
==326615==  Address 0x5f07b38 is 152 bytes inside a block of size 200 alloc'd
==326615==    at 0x483CBB5: calloc (vg_replace_malloc.c:762)
==326615==    by 0x113349: ya_setup_bar (ya_parse.c:184)
==326615==    by 0x114E4E: ya_config_parse (ya_parse.c:771)
==326615==    by 0x111F39: ya_init (ya_exec.c:430)
==326615==    by 0x1128EC: main (ya_main.c:16)
==326615==  Block was alloc'd by thread #1
==326615== 
==326615== Possible data race during write of size 8 at 0x5F0F898 by thread #7
==326615== Locks held: 1, at address 0x5F07B38
==326615==    at 0x5038F1B: _cairo_gstate_init (cairo-gstate.c:118)
==326615==    by 0x50353AC: _cairo_default_context_create (cairo-default-context.c:1489)
==326615==    by 0x110165: ya_draw_pango_text (ya_draw.c:693)
==326615==    by 0x1157D9: ya_int_date (ya_intern.c:285)
==326615==    by 0x111731: ya_exec (ya_exec.c:244)
==326615==    by 0x4840876: mythread_wrapper (hg_intercepts.c:389)
==326615==    by 0x48BB46E: start_thread (in /usr/lib/libpthread-2.31.so)
==326615==    by 0x54C83D2: clone (in /usr/lib/libc-2.31.so)
==326615== 
==326615== This conflicts with a previous write of size 8 by thread #5
==326615== Locks held: none
==326615==    at 0x503909A: cairo_list_init (cairo-list-inline.h:108)
==326615==    by 0x503909A: cairo_list_del (cairo-list-inline.h:157)
==326615==    by 0x503909A: _cairo_gstate_fini (cairo-gstate.c:208)
==326615==    by 0x5035245: _cairo_default_context_fini (cairo-default-context.c:75)
==326615==    by 0x50352B8: _cairo_default_context_destroy (cairo-default-context.c:93)
==326615==    by 0x110EA2: ya_draw_pango_text (ya_draw.c:844)
==326615==    by 0x11667B: ya_int_memory (ya_intern.c:488)
==326615==    by 0x111731: ya_exec (ya_exec.c:244)
==326615==    by 0x4840876: mythread_wrapper (hg_intercepts.c:389)
==326615==    by 0x48BB46E: start_thread (in /usr/lib/libpthread-2.31.so)
==326615==  Address 0x5f0f898 is 280 bytes inside a block of size 1,440 alloc'd
==326615==    at 0x483A7CF: malloc (vg_replace_malloc.c:309)
==326615==    by 0x50353E8: _cairo_default_context_create (cairo-default-context.c:1484)
==326615==    by 0x110165: ya_draw_pango_text (ya_draw.c:693)
==326615==    by 0x111138: ya_exec_intern_ewmh_blk (ya_exec.c:78)
==326615==    by 0x111FC6: ya_execute (ya_exec.c:445)
==326615==    by 0x1128F6: main (ya_main.c:17)
==326615==  Block was alloc'd by thread #10 
==326615== ----------------------------------------------------------------
==326615== 
==326615==  Lock at 0x5F07B38 was first observed
==326615==    at 0x4841AF9: pthread_mutex_init (hg_intercepts.c:787)
==326615==    by 0x113C23: ya_setup_bar (ya_parse.c:374)
==326615==    by 0x114E4E: ya_config_parse (ya_parse.c:771)
==326615==    by 0x111F39: ya_init (ya_exec.c:430)
==326615==    by 0x1128EC: main (ya_main.c:16)
==326615==  Address 0x5f07b38 is 152 bytes inside a block of size 200 alloc'd
==326615==    at 0x483CBB5: calloc (vg_replace_malloc.c:762)
==326615==    by 0x113349: ya_setup_bar (ya_parse.c:184)
==326615==    by 0x114E4E: ya_config_parse (ya_parse.c:771)
==326615==    by 0x111F39: ya_init (ya_exec.c:430)
==326615==    by 0x1128EC: main (ya_main.c:16)
==326615==  Block was alloc'd by thread #1
==326615== 
==326615== Possible data race during write of size 8 at 0x5F0F890 by thread #7
==326615== Locks held: 1, at address 0x5F07B38
==326615==    at 0x5038F0D: __cairo_list_add (cairo-list-inline.h:117)
==326615==    by 0x5038F0D: cairo_list_add (cairo-list-inline.h:127)
==326615==    by 0x5038F0D: _cairo_gstate_init (cairo-gstate.c:118)
==326615==    by 0x50353AC: _cairo_default_context_create (cairo-default-context.c:1489)
==326615==    by 0x110165: ya_draw_pango_text (ya_draw.c:693)
==326615==    by 0x1157D9: ya_int_date (ya_intern.c:285)
==326615==    by 0x111731: ya_exec (ya_exec.c:244)
==326615==    by 0x4840876: mythread_wrapper (hg_intercepts.c:389)
==326615==    by 0x48BB46E: start_thread (in /usr/lib/libpthread-2.31.so)
==326615==    by 0x54C83D2: clone (in /usr/lib/libc-2.31.so)
==326615== 
==326615== This conflicts with a previous write of size 8 by thread #5
==326615== Locks held: none
==326615==    at 0x5039093: cairo_list_init (cairo-list-inline.h:107)
==326615==    by 0x5039093: cairo_list_del (cairo-list-inline.h:157)
==326615==    by 0x5039093: _cairo_gstate_fini (cairo-gstate.c:208)
==326615==    by 0x5035245: _cairo_default_context_fini (cairo-default-context.c:75)
==326615==    by 0x50352B8: _cairo_default_context_destroy (cairo-default-context.c:93)
==326615==    by 0x110EA2: ya_draw_pango_text (ya_draw.c:844)
==326615==    by 0x11667B: ya_int_memory (ya_intern.c:488)
==326615==    by 0x111731: ya_exec (ya_exec.c:244)
==326615==    by 0x4840876: mythread_wrapper (hg_intercepts.c:389)
==326615==    by 0x48BB46E: start_thread (in /usr/lib/libpthread-2.31.so)
==326615==  Address 0x5f0f890 is 272 bytes inside a block of size 1,440 alloc'd
==326615==    at 0x483A7CF: malloc (vg_replace_malloc.c:309)
==326615==    by 0x50353E8: _cairo_default_context_create (cairo-default-context.c:1484)
==326615==    by 0x110165: ya_draw_pango_text (ya_draw.c:693)
==326615==    by 0x111138: ya_exec_intern_ewmh_blk (ya_exec.c:78)
==326615==    by 0x111FC6: ya_execute (ya_exec.c:445)
==326615==    by 0x1128F6: main (ya_main.c:17)
==326615==  Block was alloc'd by thread #1

mwberry avatar Mar 31 '20 05:03 mwberry

For reference, I'm running cairo-git-1.17.2+25+gaee96d175-1-x86_64.pkg.tar.xz, which already has the fix for this issue. I also tried on 1.16.0-2 and got similar results.

mwberry avatar Mar 31 '20 06:03 mwberry

Any news on this issue?

As @mwberry points out that there is a data race, I just tried to use a spinlock to protect cairo_create().

@@ -690,7 +690,9 @@ void ya_draw_pango_text(struct ya_block *blk) {
        //First override the block area with its background color in order to not draw the new text above the old one.
        xcb_poly_fill_rectangle(ya.c, blk->pixmap, blk->gc, 1, (const xcb_rectangle_t[]) { {0,0,blk->width, blk->bar->height} });
        cairo_surface_t *surface = cairo_xcb_surface_create(ya.c, blk->pixmap, ya.visualtype, blk->width, blk->bar->height);
+       pthread_spin_lock(&test_lock);
        cairo_t *cr = cairo_create(surface);
+       pthread_spin_unlock(&test_lock);

Although this seems an incorrect or partial fix, it works fine for a while. I skimmed through the cairo documentation, but I didn't find some stuffs related to thread-safety or something.

I like this project so I hope it keeps alive. Is there anybody listening to this issue? (It seems #198, #209, and #210 are the same issue)

threeearcat avatar May 16 '21 04:05 threeearcat

After I had fun with cairo things, I figure out that ya_draw_pango_text() does not need to call cairo_xcb_surface_create() and cairo_create() whenever it is called: cairo_surface_t *surface and cairo_t *cr can be created during initializing struct ya_block *blk, which seems a better solution for me. Hmm... I don't know much about cairo and X window system stuffs...

threeearcat avatar May 16 '21 07:05 threeearcat