lwindolf / liferea

Liferea (Linux Feed Reader), a news reader for GTK/GNOME
https://lzone.de/liferea
GNU General Public License v2.0
804 stars 131 forks source link

clicking 'remove item' caused a SIGSEGV #1294

Closed rich-coe closed 9 months ago

rich-coe commented 10 months ago

v1.15.0 103cb6a

In the subscription list for a feed, selected the last (newest) item. Got a SIGSEGV.

libglib-2_0-0-2.76.4-1.1.x86_64 libgtk-3-0-3.24.38-1.2.x86_64 libsoup-3_0-0-3.4.2-1.1.x86_64 libwebkit2gtk-4_1-0-2.40.4-1.1.x86_64

Thread 1 "liferea" received signal SIGSEGV, Segmentation fault.
__strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:76
76              VPCMPEQ (%rdi), %ymm0, %ymm1
(gdb) bt
#0  __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:76
#1  0x00007f15d0b2bf98 in g_strdup
    (str=0xaaaaaaaaaaaaaaaa <error: Cannot access memory at address 0xaaaaaaaaaaaaaaaa>)
    at ../glib/gstrfuncs.c:362
#2  0x00007f15d0c35179 in g_strdup_inline (str=<optimized out>) at ../glib/gstrfuncs.h:321
#3  value_collect_string
    (value=0x7ffc82323298, n_collect_values=<optimized out>, collect_values=<optimized out>, collect_flags=<optimized out>) at ../gobject/gvaluetypes.c:295
#4  0x00007f15d0c2504c in g_signal_emit_valist
    (instance=instance@entry=0x1b76ec0, signal_id=signal_id@entry=281, detail=<optimized out>, var_args=var_args@entry=0x7ffc82323438) at ../gobject/gsignal.c:3543
#5  0x00007f15d0c25bfc in g_signal_emit_by_name (instance=0x1b76ec0, detailed_signal=0x45f570 "item-updated")  
    at ../gobject/gsignal.c:3664
#9  0x00007f15d0c259df in <emit signal activate on instance 0x2b5a3f70 [GSimpleAction]>
    (instance=instance@entry=0x2b5a3f70, signal_id=<optimized out>, detail=detail@entry=0)
    at ../gobject/gsignal.c:3622
    #6  0x00007f15d0c0b448 in g_closure_invoke
    (closure=0x2a1219b0, return_value=0x0, n_param_values=2, param_values=0x7ffc82323700, invocation_hint=0x7ffc82323680) at ../gobject/gclosure.c:832
    #7  0x00007f15d0c1e4fe in signal_emit_unlocked_R
    (node=node@entry=0x1bada40, detail=detail@entry=0, instance=instance@entry=0x2b5a3f70, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7ffc82323700)
    at ../gobject/gsignal.c:3812
    #8  0x00007f15d0c2582e in g_signal_emit_valist
    (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>, var_args=var_args@entry=0x7ffc823238a0) at ../gobject/gsignal.c:3565
#10 0x00007f15d0d6743d in g_simple_action_activate (parameter=<optimized out>, action=0x2b5a3f70)
    at ../gio/gsimpleaction.c:227
#11 g_simple_action_activate (action=0x2b5a3f70, parameter=0x224f2560) at ../gio/gsimpleaction.c:207
#12 0x00007f15d0d65bab in g_action_activate (action=0x2b5a3f70, parameter=0x224f2560) at ../gio/gaction.c:399
#13 0x00007f15d140a606 in gtk_menu_tracker_item_activated (self=0x226e7150 [GtkMenuTrackerItem])
    at ../gtk/gtkmenutrackeritem.c:799
#14 gtk_menu_tracker_item_activated (self=0x226e7150 [GtkMenuTrackerItem]) at ../gtk/gtkmenutrackeritem.c:786
#18 0x00007f15d0c259df in <emit signal activate on instance 0x23c52b40 [GtkModelMenuItem]>
    (instance=instance@entry=0x23c52b40, signal_id=<optimized out>, detail=detail@entry=0)
    at ../gobject/gsignal.c:3622
    #15 0x00007f15d0c0b448 in g_closure_invoke
    (closure=0x138ba3b0, return_value=0x0, n_param_values=1, param_values=0x7ffc82323b80, invocation_hint=0x7ffc82323b00) at ../gobject/gclosure.c:832
    #16 0x00007f15d0c1e4fe in signal_emit_unlocked_R
    (node=node@entry=0x186c6e0, detail=detail@entry=0, instance=instance@entry=0x23c52b40, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7ffc82323b80)
    at ../gobject/gsignal.c:3812
    #17 0x00007f15d0c2582e in g_signal_emit_valist
    (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>, var_args=var_args@entry=0x7ffc82323d00) at ../gobject/gsignal.c:3565
#19 0x00007f15d153f194 in gtk_widget_activate (widget=0x23c52b40 [GtkModelMenuItem]) at ../gtk/gtkwidget.c:7845
#20 0x00007f15d140daa6 in gtk_menu_shell_activate_item
    (menu_shell=0x161d68f0 [GtkMenu], menu_item=0x23c52b40 [GtkModelMenuItem], force_deactivate=<optimized out>) at ../gtk/gtkmenushell.c:1375
#21 0x00007f15d140ddd9 in gtk_menu_shell_button_release (widget=0x161d68f0 [GtkMenu], event=<optimized out>)
    at ../gtk/gtkmenushell.c:791
#22 0x00007f15d129ad04 in _gtk_marshal_BOOLEAN__BOXEDv
    (closure=0x183ea20, return_value=0x7ffc82324000, instance=<optimized out>, args=<optimized out>, marshal_data=<optimized out>, n_params=<optimized out>, param_types=0x183ea50) at gtk/gtkmarshalers.c:130
#23 0x00007f15d0c0b641 in _g_closure_invoke_va
    (closure=0x183ea20, return_value=0x7ffc82324000, instance=0x161d68f0, args=0x7ffc823240d0, n_params=1, param_types=0x183ea50) at ../gobject/gclosure.c:895
#24 0x00007f15d0c24b92 in g_signal_emit_valist
    (instance=0x161d68f0, signal_id=97, detail=0, var_args=var_args@entry=0x7ffc823240d0)
    at ../gobject/gsignal.c:3472
#25 0x00007f15d0c259df in g_signal_emit
    (instance=instance@entry=0x161d68f0, signal_id=<optimized out>, detail=detail@entry=0)
    at ../gobject/gsignal.c:3622
#26 0x00007f15d154fb74 in gtk_widget_event_internal.part.0.lto_priv.0
    (widget=0x161d68f0 [GtkMenu], event=0x23907450) at ../gtk/gtkwidget.c:7812
#27 0x00007f15d13f82ee in propagate_event_up
    (topmost=<optimized out>, event=<optimized out>, widget=0x161d68f0 [GtkMenu]) at ../gtk/gtkmain.c:2588
#28 propagate_event
    (widget=widget@entry=0x23c52b40 [GtkModelMenuItem], event=event@entry=0x23907450, captured=captured@entry=0, topmost=topmost@entry=0x0) at ../gtk/gtkmain.c:2691
#29 0x00007f15d13f840c in gtk_propagate_event (event=0x23907450, widget=0x23c52b40 [GtkModelMenuItem])
    at ../gtk/gtkmain.c:2725
#30 0x00007f15d13f8dc6 in gtk_main_do_event (event=0x23907450) at ../gtk/gtkmain.c:1921
#31 gtk_main_do_event (event=<optimized out>) at ../gtk/gtkmain.c:1691
#32 0x00007f15d113f277 in _gdk_event_emit (event=0x23907450) at ../gdk/gdkevents.c:73
#33 _gdk_event_emit (event=0x23907450) at ../gdk/gdkevents.c:67
#34 0x00007f15d1193d22 in gdk_event_source_dispatch.lto_priv () at ../gdk/x11/gdkeventsource.c:354
#35 0x00007f15d0b0a988 in g_main_dispatch (context=0x17a7020) at ../glib/gmain.c:3460
#36 g_main_context_dispatch (context=context@entry=0x17a7020) at ../glib/gmain.c:4200
#37 0x00007f15d0b0ad98 in g_main_context_iterate
    (context=context@entry=0x17a7020, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>)
    at ../glib/gmain.c:4276
#38 0x00007f15d0b0ae2c in g_main_context_iteration
    (context=context@entry=0x17a7020, may_block=may_block@entry=1) at ../glib/gmain.c:4343
#39 0x00007f15d0d5e83d in g_application_run
    (application=0x17a48c0 [LifereaApplication], argc=argc@entry=6, argv=argv@entry=0x7ffc823245b8)
    at ../gio/gapplication.c:2573
#40 0x000000000042d2d8 in liferea_application_new (argc=6, argv=0x7ffc823245b8) at ../liferea_application.c:343
#41 0x00007f15d07f0bf0 in __libc_start_call_main
    (main=main@entry=0x41f550 <main>, argc=argc@entry=6, argv=argv@entry=0x7ffc823245b8)
    at ../sysdeps/nptl/libc_start_call_main.h:58
#42 0x00007f15d07f0cb9 in __libc_start_main_impl
    (main=0x41f550 <main>, argc=6, argv=0x7ffc823245b8, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7ffc823245a8) at ../csu/libc-start.c:360
#43 0x000000000041f5f5 in _start () at ../sysdeps/x86_64/start.S:115
Deepfriedice commented 10 months ago

Apologies if I'm butting in here. I don't really know the process for contributing but I thought I'd try to help since this issue is bothering me.

This looks like it's caused by the ordering of events in the itemlist_remove_item function in itemlist.c:449 :

/* hard unconditional item remove */
void
itemlist_remove_item (itemPtr item)
{
    if (itemlist->priv->selectedId == item->id) {
        itemlist_set_selected (NULL);
        itemlist->priv->deferredRemove = FALSE;
    }

    itemlist_duplicate_list_remove_item (item);

    itemview_remove_item (item);
    itemview_update ();

    db_item_remove (item->id);

    /* update feed list counters*/
    vfolder_foreach (node_update_counters);
    node_update_counters (node_from_id (item->nodeId));

    item_unload (item);
    g_signal_emit_by_name (itemlist, "item-updated", item->nodeId);
}

item_unload seems to be just an alias for g_object_unref (item.h:106), so the last two lines here are decrementing the reference count (which seems to reliably trigger a free) and then trying to access the nodeId field. I belive nodeId is getting overwitten after the free, and then g_signal_emit_by_name is trying to dereference it, casuing the segfault. I don't know what g_signal_emit_by_name is responsible for here, but I suspect these last two lines can be swapped, so that we emit the signal first and then free the _LifereaItem struct? I tried this locally and it seems to work, but I'm not sure how to run Liferea's tests.

rich-coe commented 10 months ago

@Deepfriedice I'll reverse the lines and re-try it. Thanks!

rich-coe commented 10 months ago

@lwindolf Can you comment on this set of lines in itemlist.c::itemlist_select_from_history()

    itemview_select_item (item);
    item_unload (item); 

It does not seem to be invoked when I click next. I'm not sure what activates 'on_next_read_item_activate' from the UI. Is it correct to unload the item after selecting it?

lwindolf commented 9 months ago

@Deepfriedice Thank you for the hint. I reverse both lines!

lwindolf commented 9 months ago

@rich-coe I believe the item_unload() to be correct at this place as itemview_select_item() does not free the item.

As for who calls itemlist_select_from_history() it is called by the first two menu items in the "Items" menu (or with headerbar active the left and right arrow button).