Skip to content

Fix astroid webextension process crashing in thread view#772

Draft
ibuclaw wants to merge 3 commits into
astroidmail:masterfrom
ibuclaw:fix_libtv_crash
Draft

Fix astroid webextension process crashing in thread view#772
ibuclaw wants to merge 3 commits into
astroidmail:masterfrom
ibuclaw:fix_libtv_crash

Conversation

@ibuclaw
Copy link
Copy Markdown
Contributor

@ibuclaw ibuclaw commented May 19, 2026

Not sure what caused this to occur, looking at local system upgrade history, webkit2gtk recently got updated from 2.50.4 to 2.52.3.

To reproduce, open any email in a new thread view, and the new tab is just a blank white page that doesn't respond to any keybindings. Closing astroid then results in a segfault inside the dtor for PageClient.

First sign that something is going wrong was this error in the console.

(process:170163): Gtk-CRITICAL **: 18:16:38.827: gtk_icon_theme_get_for_screen: assertion 'GDK_IS_SCREEN (screen)' failed

Which occurs inside the call to Gtk::IconTheme::get_default();
https://github.com/astroidmail/astroid/blob/master/src/modes/thread_view/webextension/tvextension.cc#L133

Subsequently, the return value assigned to theme is NULL. Which then results in the web extension thread dying due to the null dereference on the next line of code.

As far as I can tell, this static method expects the default screen to have already been opened, and adding an assert confirmed that no display is open inside the web extension thread.

g_assert (Gdk::Display::get_default ());

$ ./build/astroid
...
ERROR:astroid/src/modes/thread_view/webextension/tvextension.cc:132:AstroidExtension::AstroidExtension(WebKitWebExtension*, gpointer): assertion failed: (Gdk::Display::get_default ())
Bail out! ERROR:astroid/src/modes/thread_view/webextension/tvextension.cc:132:AstroidExtension::AstroidExtension(WebKitWebExtension*, gpointer): assertion failed: (Gdk::Display::get_default ())

This change reflects some observations from looking within gtk and gdk itself.

https://github.com/GNOME/gtkmm/blob/2160941ea7e63daa74782e53c7089659b5628293/gtk/gtkmm/init.cc#L28

https://github.com/GNOME/gtk/blob/0e4930b10bdee987d1e79cdc11494c953f621433/gdk/gdk.c#L371

ostream.clear ();
if (ready) {
istream.clear ();
ostream.clear ();
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping this here, as race condition is still possible, even though you'd have to close astroid within milliseconds of opening a new thread view.

@ibuclaw ibuclaw force-pushed the fix_libtv_crash branch 2 times, most recently from ce5d648 to 9d0d35b Compare May 19, 2026 17:03
@gauteh
Copy link
Copy Markdown
Member

gauteh commented May 19, 2026

Looks good. Is it ready to merge? The debian CI may be old, and ready for disabling?

@ibuclaw
Copy link
Copy Markdown
Contributor Author

ibuclaw commented May 19, 2026

Looks good. Is it ready to merge? The debian CI may be old, and ready for disabling?

CI does need updating indeed. I wonder if the tests might trip the issue I'm seeing if tried on a newer distro version.

@ibuclaw ibuclaw force-pushed the fix_libtv_crash branch 6 times, most recently from 956d1ae to 20a2055 Compare May 20, 2026 00:31
@ibuclaw
Copy link
Copy Markdown
Contributor Author

ibuclaw commented May 20, 2026

@ibuclaw ibuclaw force-pushed the fix_libtv_crash branch from 20a2055 to c7f70e3 Compare May 20, 2026 00:38
@ibuclaw ibuclaw marked this pull request as draft May 20, 2026 00:58
@ibuclaw
Copy link
Copy Markdown
Contributor Author

ibuclaw commented May 20, 2026

Still failures on ubuntu noble, but curiously not on ubuntu resolute.

https://github.com/astroidmail/astroid/actions/runs/26134040134/job/76865149422?pr=772#step:7:176

Can't reproduce locally, but maybe there's still missing initialisation?

@ibuclaw ibuclaw force-pushed the fix_libtv_crash branch 2 times, most recently from ac0a606 to c7f70e3 Compare May 20, 2026 01:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants