Very Secure

ircbot no suicide on reconnect

ircbot-no-suicide-on-reconnect.vpatch
ircbot-no-suicide-on-reconnect.vpatch.whaack.sig

While writing fleetbot, whose functionality requires maintaining a whole lot of connections, I ran into a bug with ircbot's reconnection logic. The bug likely has stayed hidden because it manifests only when a specific thread calls ircbot-reconnect.1

The explanation of the bug is as follows:

Ircbot uses two threads. One thread is the run-thread that reads messages from the irc network and dispatches them to functions. The other thread, the ping-thread, sends a ping to the server every *ping-freq* seconds. The ping-thread also calls ircbot-reconnect if ircbot has not received a pong in *max-lag* seconds. ircbot-reconnect first calls ircbot-disconnect, which closes the connection and terminates the ping-thread. But since the active thread is the ping-thread when ircbot-disconnect is called, the ping-thread terminates itself. Thus the call to ircbot-reconnect never reaches the "connect" step of reconnection.

This vpatch changes ircbot so that ircbot-disconnect does not terminate the ping-thread if the ping-thread is nil or the active thread is the ping-thread. So now when the ping-thread successfully reconnects, it becomes the run-thread and spawns a new ping-thread to do its old job.2

In this patch I included two other changes:

1. I removed channel reconnection on kick. It's not explicitly stated in the #trilema bot spec, but I get the impression that bots are to do as told. If someone with the authority to do so kicks a bot, the bot should not fight back.

2. I fixed a misnamed parameter in make-ircbot.

1 diff -uNr a/botworks/ircbot/ircbot.lisp b/botworks/ircbot/ircbot.lisp
2 --- a/botworks/ircbot/ircbot.lisp c74e827a6c6c5cc4baf2bb72564e02a10057fd608cb1b11e504f81730ac30e2692bf54f5735faa6e4bd69be9f1ec9837cf12608551c0936653cf2880a54e2a6a
3 +++ b/botworks/ircbot/ircbot.lisp 9741409ae7fa93d709c729335c5396899d1b8f8f376d8fdd5284d9f1ddd073d1db49419fa859efe6b540c979990f16070137e849ee11e1ff57405ecdbc48a6cd
4 @@ -40,11 +40,6 @@
5 (add-hook conn 'irc-err_nicknameinuse-message (lambda (message)
6 (declare (ignore message))
7 (ircbot-randomize-nick bot)))
8 - (add-hook conn 'irc-kick-message (lambda (message)
9 - (declare (ignore message))
10 - (map nil
11 - (lambda (c) (join (ircbot-connection bot) c))
12 - (ircbot-channels bot))))
13 (add-hook conn 'irc-notice-message (lambda (message)
14 (ircbot-handle-nickserv bot message)))
15 (add-hook conn 'irc-pong-message (lambda (message)
16 @@ -65,7 +60,8 @@
17 (setf (ircbot-connection bot) nil)
18 (if (not (null (ircbot-run-thread bot)))
19 (sb-thread:terminate-thread (ircbot-run-thread bot)))
20 - (sb-thread:terminate-thread (ircbot-ping-thread bot))))
21 + (if (not (or (null (ircbot-ping-thread bot)) (equal sb-thread:*current-thread* (ircbot-ping-thread bot))))
22 + (sb-thread:terminate-thread (ircbot-ping-thread bot)))))
23
24 (defmethod ircbot-reconnect3
25 (let4)))
26 @@ -133,10 +129,10 @@
27 do (return t)))
28
29
30 -(defun make-ircbot (server port nick password channel)
31 +(defun make-ircbot (server port nick password channels)
32 (make-instance 'ircbot
33 :server server
34 :port port
35 :nick nick
36 :password password
37 - :channel channel))
38 + :channels channels))
39 diff -uNr a/botworks/manifest b/botworks/manifest
40 --- a/botworks/manifest b4d189ecf4e93ea7539d9512dc7554c44ca759712e0b035f04a850f508c84a244dae778387edd6e6a463204d35bf3ed8246ded018581cd4b8347189948cef643
41 +++ b/botworks/manifest 22d9a0246e251298ccfa31918fc9193b306aa202bf103ac2ad92b9920c40eb5cca7b10fa4e400e92b2b39a8a2483fe7d2a0295add1ce548d66b9373c12fa4928
42 @@ -1,2 +1,3 @@
43 424545 ircbot-genesis trinque ircbot genesis, http://trinque.org/2016/08/10/ircbot-genesis/
44 447104 ircbot-multiple-channels-corrected ben_bulpes multiple channel support for ircbot, http://cascadianhacker.com/correction-multiple-channel-patches-for-irclogbot
45 +615488 ircbot-no-suicide-on-reconnect whaack fixes reconnect bug + removes rejoin on chan kick, http://ztkfg.com/2020/02/ircbot-no-suicide-on-reconnect/
46

Although not included in this vpatch, ircbot would benefit from a better reconnection strategy. Currently it attempts to reconnect only once. And since the bot detected lag prior to the moment it attempts to reconnect, it is likely that that the one reconnect attempt will not succeed. So although this vpatch fixes a logical error, I am doubtful that it will increase ircbot's uptime by much. I will submit another vpatch with a more aggressive reconnection strategy once I have fleshed out all the details for its use case with fleetbot.

  1. This bug will be invisible to someone calling ircbot-reconnect in the repl. []
  2. This is not the case if one started ircbot by running ircbot-connect-thread. In that situation, the ping-thread calls ircbot-connect-thread and terminates. ircbot-connect-thread creates a new run-thread as well as a new ping-thread. []
  3. bot ircbot) &optional (quit-msg "..." []
  4. threaded-p (not (null (ircbot-run-thread bot []

2 Responses to “ircbot no suicide on reconnect”

  1. [...] I signed my first vpatch and published the corresponding article. [...]

  2. [...] to my knowledge, I posted it in a paste at some point and I suppose it got lost meanwhile. I know Whaack was looking into it, while Trinque is, as far as I know, maintaining his own [...]

Leave a Reply