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

Assertion failure when sending an invalid NICK #466

Closed
LemonBoy opened this issue Apr 2, 2016 · 5 comments
Closed

Assertion failure when sending an invalid NICK #466

LemonBoy opened this issue Apr 2, 2016 · 5 comments
Milestone

Comments

@LemonBoy
Copy link
Member

LemonBoy commented Apr 2, 2016

How to reproduce:

  • Set the nickname to 38952c30
  • Connect to Freenode
  • Watch the world burn

What happens:

We send an invalid nick and the server answers with a 432 that gets handled in irc-nicklist.c by just truncating the connection. sig_disconnect is then reached which clears the isupport hashtable.
Then we try to print the raw message from the server and call ischannel_func, g_hash_table_lookup fails and emits those warnings.

@ailin-nemui
Copy link
Contributor

why is the disconnect handled before the event 432?

@LemonBoy
Copy link
Member Author

LemonBoy commented Apr 3, 2016

I do see Irssi: Connection lost to chat.freenode.net in the status window, we probably read the remaining data from the socket, then get booted and actually process the data after the party's already over

@ailin-nemui ailin-nemui added the bug label Apr 4, 2016
@ailin-nemui
Copy link
Contributor

looking for ideas to improve the irssi event system... currently the 432 will post a disconnect signal but I would prefer that to be run after all the other event code has finished to avoid this situation

server event
-> event 432
     -> emits disconnect
     other event 432 codes run (can be avoided with signal_continue)
other server event code runs (not easily avoided) <-- need ideas here

@dequis
Copy link
Member

dequis commented Apr 17, 2016

Related: 8312144

-       signal_add_first("server event", (SIGNAL_FUNC) irc_server_event);
+       signal_add_last("server event", (SIGNAL_FUNC) irc_server_event);

That's the irc_server_event of channel-events.c, not the one from irc.c.

The one from irc.c is the one which calls server_disconnect, which runs first. Backtrace looks sorta like this, adding the event_nick_invalid frame back.

#0 server_disconnect (server=...) at servers.c:466
#1 event_nick_invalid (server=..., data="* 38952c30 :Erroneous Nickname")
   at irc-nicklist.c:316
#2 signal_emit_real (rec=..., params=4, va=..., first_hook=...) at signals.c:242
#3 signal_emit (signal=..., params=4) at signals.c:286
#4 irc_server_event (server=..., line="432 * 38952c30 :Erroneous Nickname",
   nick="hitchcock.freenode.net", address=0x0) at irc.c:308

The irc_server_event from channel-events.c happens afterwards (added with signal_add) and this:

#0 g_log ()
#1 g_hash_table_lookup ()
#2 ischannel_func (server=..., data="38952c30") at irc-servers.c:87
#3 irc_channel_find_server (server=..., channel="38952c30") at irc-channels.c:181
#4 check_join_failure (server=..., channel="38952c30") at channel-events.c:39
#5 irc_server_event (server=..., line=...) at channel-events.c:64

But apparently there was a good reason to give the other one higher priority. Maybe it should just stop the "server event" signal if it disconnected after emitting "event XXX"?

Attached debugging session: gdb.txt

@dequis
Copy link
Member

dequis commented Jan 5, 2017

77aab79

@dequis dequis closed this as completed Jan 5, 2017
@ailin-nemui ailin-nemui added this to the 0.8.21 milestone Jun 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants