-
-
Notifications
You must be signed in to change notification settings - Fork 179
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.
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
Roster assertion failure kills profanity with SIGABRT #1083
Comments
Custom theme that I run Profanity with can be found at: https://gist.github.com/ccxcz/100c5fbbadba6ecaba2feb6f1c0f7465 |
Looks like this is use after free issue. Roster is freed on connection loss and |
I've tested with c94f3d0 and the problem I get now is that on reconnect the rooms now don't get rejoined properly and if I try to join a room manually I get a crash that seems to indicate it tries to switch to the window without checking it exists:
|
From usability perspective I find closing the windows on connection drop rather inconvenient, especially if they are to be rejoined after reconnect. It means all the conversations get purged on every network hiccup. Sure, I still get logs (if I enable them), but it definitely worsens usability of running Profanity in the background and then coming back to read the backlog. |
To reproduce this bug, change preferences:
and cause a connection loss (autoping or broken connection). After the connection loss roster is destroyed, but it is accessed during rewriting UI to calculate width for statusbar. To solve this we must not take window's name from the roster. Rather save it in the window object. |
With 48013f8 I changed the statusbar setting. Closed my Laptops lid, opened it after a while and got this: |
Save the name for displaying the windows in the statusbar inside the tab object. So far we calculated them repeatedly and this created issues when we lost the connection. Regards #1083
We destory the roster in ev_disconnect_cleanup(). Adding a function to test if the roster has been destroyed and testing for it in the statusbar. So now when the connection is lost 'Lost connection' is printed in all open windows. We can then reconnect with `/connect accountname`. Should fix #1083
@ccxcz can you please test the new PR? |
We destory the roster in ev_disconnect_cleanup(). Adding a function to test if the roster has been destroyed and testing for it in the statusbar. So now when the connection is lost 'Lost connection' is printed in all open windows. We can then reconnect with `/connect accountname`. Should fix #1083
Some people tested the PR and said it worked. However I'll leave this open for @ccxcz to reply. |
The crashes are gone indeed. There is ongoing problem with MUCs not getting reconnected properly, but that's deserving of it's own issue. |
Built profanity from commit 722cb5f
This happens randomly with no activity from user by just being connected to several active MUCs.
The text was updated successfully, but these errors were encountered: