FS#65885 - [libappindicator-gtk3] Segfault with Discord

Attached to Project: Community Packages
Opened by Paul G (paulieg) - Wednesday, 18 March 2020, 23:36 GMT
Last edited by Balló György (City-busz) - Friday, 03 July 2020, 14:40 GMT
Task Type Bug Report
Category Upstream Bugs
Status Closed
Assigned To Levente Polyak (anthraxx)
Architecture All
Severity Medium
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 6
Private No

Details

Description:

Several popular applications, such as Discord, have been suffering from constant crashes for years due to a bug in libappindicator. See for example: https://github.com/flathub/com.discordapp.Discord/issues/30

I have found this bug, fixed it and submitted the bug report and patch 'upstream' here:

https://bugs.launchpad.net/archlinux/+source/libappindicator/+bug/1867996

I say 'upstream' because the only place I was able to find to report it is Ubuntu's bugtracker, which isn't strictly speaking upstream for libappindicator. However, a real upstream is unavailable as its launchpad page doesn't accept bug reports.

Given this, it is unlikely that the patch will be reviewed, applied and released any time soon. In the spirit of having Arch work properly when borked messes like Ubuntu don't, I propose applying my patch during the package build process.
Attached please find a) the patch itself and b) the patch to the PKGBUILD. I have tested the new PKGBUILD and attached the patch so it is as easy as possible for the package maintainer to roll out this fix.

This task depends upon

Closed by  Balló György (City-busz)
Friday, 03 July 2020, 14:40 GMT
Reason for closing:  Fixed
Additional comments about closing:  libappindicator-gtk3 12.10.0.r296-1
Comment by Balló György (City-busz) - Thursday, 19 March 2020, 08:21 GMT
The patch you sent is not correct. It breaks the signal, but won't fix the underlying code.

Please post a proper trace as described here:
https://wiki.archlinux.org/index.php/Debug_-_Getting_Traces
Comment by Paul G (paulieg) - Thursday, 19 March 2020, 14:18 GMT
You're right in that the patch is wrong - it fixes *a* bug (of the same type, elsewhere), not *the* bug I was trying to fix. No idea how that's happened. Mea maxima culpa and not acceptable.

The problem appears more widespread - multiple emits of g_signal pass in an extra boolean vararg at the end that is not declared in the definition of the signals.


Examples:
---

definition:
signals[NEW_ICON_THEME_PATH] = g_signal_new (APP_INDICATOR_SIGNAL_NEW_ICON_THEME_PATH,
G_TYPE_FROM_CLASS(klass),
G_SIGNAL_RUN_LAST,
G_STRUCT_OFFSET (AppIndicatorClass, new_icon_theme_path),
NULL, NULL,
g_cclosure_marshal_VOID__STRING,
G_TYPE_NONE, 1, G_TYPE_STRING);
call:
g_signal_emit (self, signals[NEW_ICON_THEME_PATH], 0, self->priv->icon_theme_path, TRUE);


definition:

signals[NEW_ICON] = g_signal_new (APP_INDICATOR_SIGNAL_NEW_ICON,
G_TYPE_FROM_CLASS(klass),
G_SIGNAL_RUN_LAST,
G_STRUCT_OFFSET (AppIndicatorClass, new_icon),
NULL, NULL,
g_cclosure_marshal_VOID__VOID,
G_TYPE_NONE, 0, G_TYPE_NONE);

call:
g_signal_emit (self, signals[NEW_ICON], 0, TRUE);


Unless I'm fundamentally misunderstanding something (which is possible, since this is the first time I've looked at g_signals and gobject in general), these signalls clearly don't expect the G_TYPE_BOOL they are getting, hence the segfaults in va_list handlng.
Comment by Paul G (paulieg) - Thursday, 19 March 2020, 20:47 GMT
New patch. I've gone through the code and audited g_signal definitions and calls to g_signal_emit, correcting both where they were incorrect. Wrote up a simple test program to test that NEW_ICON and NEW_ATTENTION_ICON still work (they do) and used blueman-tray to test other functionality.
Comment by Balló György (City-busz) - Thursday, 19 March 2020, 21:15 GMT
I don't think that it's a problem with the signals. We don't know what actually happens until you don't post a proper trace (e.g. rebuilding libappindicator-gtk3 with debug option, and then read the trace with coredumpctl).
Comment by Ian Whitlock (gigawhitlocks) - Friday, 20 March 2020, 16:14 GMT
I believe I'm hitting the same bug in a different Electron application, Mattermost Desktop, and I have a backtrace that I can provide (attached). The debug info is available for the system libraries that Electron calls into in this trace, so I think it will be more useful than the one in the original report.

I'm not running Arch anymore but this bug seems to be distribution-agnostic. Please let me know if I can help in any other way resolving this bug.
Comment by Paul G (paulieg) - Friday, 20 March 2020, 17:41 GMT
It is distro-agnostic, yes, the only reason I filed here in addition to -as-upstream-as-i-could-find is that I use Arch and don't want to have to maintain local-only patchsets if at all avoidable.
If you get these crashes often enough to be "reproducible", try building a separate copy of libappindicator with this patch applied, and LD_PRELOAD it for the electron app in question. If the crashes go away, it's an extra datapoint.
Comment by Ian Whitlock (gigawhitlocks) - Friday, 20 March 2020, 18:23 GMT
> If you get these crashes often enough to be "reproducible", try building a separate copy of libappindicator with this patch applied, and LD_PRELOAD it for the electron app in question

I did this last night and left my application open and still witnessed the crash some time overnight :(
Comment by Paul G (paulieg) - Friday, 20 March 2020, 21:14 GMT
Hrmf. What does gdb bt full on the coredump say?
Comment by Ian Whitlock (gigawhitlocks) - Friday, 20 March 2020, 21:45 GMT
Attached the output from bt full in gdb for you to take a look. Just to be clear, this is from a coredump from an execution run where the patch was in place. Thank you for your effort looking into this.
Comment by Ian Whitlock (gigawhitlocks) - Monday, 23 March 2020, 20:53 GMT
Applying this patch in addition to your segfault-fix.patch seems to have fixed the segfaults, for me at least.

https://bugs.launchpad.net/archlinux/+source/libappindicator/+bug/1867996/+attachment/5339867/+files/n_elements.patch
Comment by Thor (ekkelett) - Saturday, 18 April 2020, 20:24 GMT
A new version of libappindicator was released. Because of this I flagged the current libappindicator-gtk3 package as out-of-date.

Loading...