Skip to content
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

try to make sure the server is still good enough to call ischannel when printing netsplit/join #832

Merged
merged 1 commit into from Feb 12, 2018

Conversation

Projects
None yet
2 participants
@ailin-nemui
Copy link
Contributor

ailin-nemui commented Jan 25, 2018

This should fix #819

@ailin-nemui ailin-nemui force-pushed the ailin-nemui:netsplit-crash branch 2 times, most recently from 4673d7e to 71ec537 Jan 25, 2018

@dequis

This comment has been minimized.

Copy link
Member

dequis commented Jan 29, 2018

Continuing #819 (comment) here:

This throws two warnings:

22:47 -!- Irssi: critical ischannel_func: assertion 'data != NULL' failed
22:47 -!- Irssi: Disconnecting from server localhost: [Reconnecting]
22:47 -!- Irssi: critical ischannel_func: assertion 'data != NULL' failed

It also seems to not flush pending netsplits like lemon-netsplit-base does

22:47 <@dx|fb> ac off
22:47 <@root> Deactivating all active (re)connections...
22:47 <@root> facebook - Signing off..
22:47 -!- dx|fb [dx3@localhost.localdomain] has joined &bitlbee

lemon-netsplit-base (and #824) looks like this:

23:15 <@dx|fb> ac off
23:15 <@root> Deactivating all active (re)connections...
23:15 <@root> facebook - Signing off..
23:15 -!- Netsplit localhost.localdomain <-> facebook.localhost.localdomain quits: +fb-XXXX, +fb-XXXX, +fb-XXXX, +fb-XXXX, +fb-XXXX, +fb-XXXX
23:15 -!- dx|fb [dx3@localhost.localdomain] has joined &bitlbee
@ailin-nemui

This comment has been minimized.

Copy link
Contributor Author

ailin-nemui commented Jan 29, 2018

how about we change the check to dest==NULL || server_ischannel

@ailin-nemui

This comment has been minimized.

Copy link
Contributor Author

ailin-nemui commented Feb 5, 2018

I'm proposing to put this into git (been running it for a while and tested with bitlbee) @irssi/developers

@dequis

This comment has been minimized.

Copy link
Member

dequis commented Feb 5, 2018

So the safer one for 1.1.1, and this one for git? Sounds like a plan.

There's still the issue that this won't flush pending netsplits when quitting, but that's not a big deal. The real question is if the circumstances that lead to netsplits not being flushed on quit (non-null server, null target) can apply to other less situations in the middle of the connection.

@ailin-nemui

This comment has been minimized.

Copy link
Contributor Author

ailin-nemui commented Feb 5, 2018

in my testing it did flush them, can you double check that? we can also add a server disconnected handler and add an explicit flush, maybe that's better? I'll rebase this tomorrow....

@ailin-nemui ailin-nemui force-pushed the ailin-nemui:netsplit-crash branch from 8567cc5 to 9f6b9f3 Feb 9, 2018

@ailin-nemui

This comment has been minimized.

Copy link
Contributor Author

ailin-nemui commented Feb 9, 2018

@ailin-nemui ailin-nemui force-pushed the ailin-nemui:netsplit-crash branch from 9f6b9f3 to 5c5ed64 Feb 9, 2018

@ailin-nemui ailin-nemui merged commit ccfb2da into irssi:master Feb 12, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ailin-nemui ailin-nemui deleted the ailin-nemui:netsplit-crash branch Feb 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.