Commit 41390bfe5f fixed a bug whereby the processing
of a MODRESTART command could result in a crash. The approach
taken in this fix was to defer the reloading of all modules
so that the call stack does not contain functions located in
modules that are being reloaded. It did this by scheduling a
one-shot timer event for 1 second in the future, in the absense
of any better deferral mechanism at the time. Timers are
processed by the event loop, which is core to IRCd and cannot
be reloaded.
Commit 59ea3c6753 introduced a mechanism to defer the
execution of a function until all events have been processed by
the event loop, in order to fix a REHASH bug that could result
in a crash due to closing and reopening listener sockets with a
pending socket connection event to process after the REHASH was
completed.
Rework commit 41390bfe5f to use the new deferral
mechanism introduced by commit 59ea3c6753 and do the
same for module reloads.
This allows reusing this function for other uses that just need to
remove this client from others' accept lists on nick change and not have
duplicates of this code everywhere
* move has_common_channel to s_user.c
* don't remove clients from /accept on NICK when there's a common channel
Co-authored-by: Ed Kellett <e@kellett.im>
We were ending the flood grace period for any channel mode command other
than `MODE #foo [bq]` by means of a hardcoded check. I've moved that to
after we parse the mode string, so we can correctly identify all
requests to change modes and end the grace period on exactly those.
It would have been entirely possible to move the check even further down
and flood_endgrace on only mode commands that *actually* change modes,
but I don't like the idea of making it sensitive to external conditions.
I'm preparing to PR a succession of privs changes with the ultimate goal
of severely limiting the scope of the binary oper/user dichotomy and
move conceptually distinct oper functions into their own privs.
Accomplishing this is a non-trivial task, and can wait, but it's
inconvenient now to have such functions enabled by the same mechanism
that grants any privs at all--so I'm moving all of them to a
transitional priv with the intention of eroding that later.
/modrestart used to be implemented as a normal command and could crash
when used remotely because it would reload m_encap, which was on the
call stack at the time. This was fixed in 41390bfe5f. However,
/modreload has exactly the same problem, so I'm giving it the
same treatment.
Incidentally: This bug was first discovered in ircd-seven, where the
`/mod*` commands themselves live in the core, so m_encap was the only way
the crash could happen (and it didn't most of the time, because m_encap
would only be moved if you got unlucky). But `/mod*` are in modules in
charybdis, so /modrestart would have unloaded the code it was in the
middle of executing. With that in mind, I'm not sure how it ever
appeared to work.
Reloading modules sends CAP DEL followed by an immediate CAP NEW:
:staberinde.local CAP * DEL :account-tag
:staberinde.local CAP * NEW :account-tag
This isn't very nice. /modrestart is particularly bad. In order to avoid
doing this, we remember the capability set at the beginning of module
operations, compare that with the set afterwards, and report only the
differences with CAP {DEL,NEW}.
core/m_server.c:138:3: warning: 'break' will never be executed
[-Wunreachable-code-break]
(... and 3 more of the same)
Why put an unreachable comment in the code *and then write a
statement following it* ? O_o
