core: fix broken interrupt handling #1527
Conversation
|
grammar peeve 😢... will change connection --> connecting |
|
I'd like to fix this in the 6.6.x release series, but am wary of the merge conflicts it would create because this fix would need to happen on We're still at a point where fixes from |
But its sooo worth it! 😝 Funnily enough, I initially based this fix on an old |
|
Oh hell, why not throw it in for |
|
woohoo! Seriously though, I only say this b/c of #1478 (comment) |
|
Time to make a new issue label here called "Affects: Docker" /s |
There was a problem hiding this comment.
Approved #1534? Approve #1527. They're equivalent.
If I'm feeling masochistic I'll merge this first, then the 6.6.x branch with its version of this fix. If not, I'll make you do the rebase @HumorBaby. 😜
|
I'm honestly quite impressed by how small the changes are. And it looks good so far! |
|
Sad news 😢 I found another case where exit can be messy during testing for #1543: during the connection phase before the connection is accepted. Test this by trying to connect to a server on a wrong port (where it is most likely not to be accepted, but not explicitly rejected). |
|
Worth making a quick PR for 6.6.x so this can be fixed in the 6.6.6 release on Monday? |
|
Yep. |
HumorBaby
force-pushed
the
1478-fix-broken-interrupt-handling
branch
from
3177610 to
7a3d66a
Compare
|
@HumorBaby Would you be put out if I asked to reference 9dd772d in this commit message? Like: Also, in "Because the Optionally, also put a reference to #1493 in the message, since that's why this needs to be reapplied. |
Because of the `time.sleep` during the disconnect phase, the signal handling is thrown off and a new bot is spawned before the `bot.hasquit` flag can be checked. Add a check at the top of the loop to see if the `hasquit` flag was set during a disconnect. Fixes sopel-irc#1478. Reapplied because of changes from sopel-irc#1493.
HumorBaby
force-pushed
the
1478-fix-broken-interrupt-handling
branch
from
7a3d66a to
ec9eeb7
Compare
Too much. 🙃 |
|
Perfect! |
@dgw did a good job describing the issue in #1478, so I refer the reviewer there for the details.
The fixes provided here use simple checks to ensure 1) that a socket is present before attempting to send a
QUITmessage, and 2) ahasquitflag was not set on a bot instance during a disconnected phase.Generally, in both cases, the issue was the signal setting the
hasquitflag too late, and allowing a newwhileloop to begin. The bot would still have thehasquitflag set, but this would not be caught until a newException/signal.