• ircd: fix netsplit stack overflow on deep server topologies

    From Rob Swindell@VERT to GitLab note in main/sbbs on Sat Aug 1 02:03:32 2026
    https://gitlab.synchro.net/main/sbbs/-/merge_requests/714#note_9845

    Context that was missing when this was written: the `too much recursion` error you were given came from a specific incident on cvs.synchro.net, but it was pasted without the surrounding log, so there was no way to see what triggered it. I went back and root-caused it. Short version: the stack depth is a real bug and this patch fixes it, but it is not what caused that error, and on the input that did cause it this patch hangs instead of crashing.

    ### What actually happened

    The server's `[Info] Servername` was wrong (a config conversion had frozen in another host's name), so its legitimate `[Server]` link to that host became a link bearing *its own* name. `Register_Unregistered_Local_Server()` sets `linkparent = ServerName` on every local link, so that link ended up with `nick` and `linkparent` both equal to `ServerName`. `netsplit()` quits every server whose `linkparent` matches `this.nick` -- which, for that link, is itself. quit -> netsplit -> quit until the stack ran out. Two servers were involved, not a deep tree.

    ### Measurements

    I drove `IRCClient_netsplit()` / `Server_Quit()` from each revision against synthetic topologies:

    | scenario | before | this MR | + visited set |
    |---|---|---|---|
    | chain, 1400 deep | ok | ok | ok |
    | chain, 1600 deep | too much recursion | **ok** | ok |
    | self-named link (the incident) | too much recursion | **hangs** | ok |
    | 2-node linkparent cycle | too much recursion | **hangs** | ok |

    So the premise holds and I was wrong to doubt it: the recursion really does exhaust the stack, somewhere between 1400 and 1600 levels. That is worth
    fixing on its own and nothing else in flight addresses it.

    ### The blocking issue

    The BFS queue has no visited set, and `Servers[i].linkparent == srv` matches a server whose `linkparent` is its own nick, so it pushes the same name forever. `queue` grows without bound and the removal loop after it never runs. Against
    a live reproduction the daemon does not die, it wedges: the port still
    accepts connections, no client ever gets `001`, memory climbing. That is worse operationally than the crash, because a crash gets noticed and restarted while a hang looks alive.

    Six lines fix it. I built and tested this on top of your commit; it passes the depth cases above and an end-to-end reproduction of the incident:

    ```js
    queue = [this.nick];
    seen = {};
    seen[this.nick.toLowerCase()] = true;
    for (j = 0; j < queue.length; j++) {
    ...
    for (i in Servers) {
    if ( Servers[i]
    && Servers[i].linkparent == srv
    && !seen[Servers[i].nick.toLowerCase()]
    ) {
    seen[Servers[i].nick.toLowerCase()] = true;
    queue.push(Servers[i].nick);
    }
    }
    }
    ```

    Seeding `seen` with `this.nick` covers the self-loop, and index 0 stays
    skipped in the removal loop so `Server_Quit()` still deletes the root itself.

    ### Minor

    Child servers no longer pass through `Server_Quit()`, so a **local** child skips its socket close, sendq/recvq purge, Y:Line decrement, `client_remove()` and ping-interval clear. In a well-formed topology children are always remote and this is inert, but the patch deletes from `Local_Servers[]`, so local children appear to have been contemplated.

    ### Merging

    Clean against master today. There is unpushed work on the same function (refusing a link that carries our own server name, plus a re-entrancy guard in `Server_Quit()`), which will conflict. Suggest landing the visited set with this MR and the link rejection alongside it: the visited set stops the daemon dying, and refusing the link is what stops the malformed topology existing in the first place.

    -- *Authored by Claude (Claude Code), on behalf of @rswindell*

    ---
    þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net
  • From Rob Swindell@VERT to GitLab note in main/sbbs on Sat Aug 1 02:07:06 2026
    https://gitlab.synchro.net/main/sbbs/-/merge_requests/714#note_9846

    Heads-up: the work I described above as unpushed is now on master, so this MR has gone to `cannot_be_merged`. Sorry for the moving target -- that sentence was accurate when I wrote it and is not any more.

    The conflict is `IRCClient_netsplit()` only, from
    22ae12212b (appears-22-intense, 2026-08-01), which refuses a link carrying our own server name and adds a re-entrancy guard to `Server_Quit()`. It has one change inside the function itself:

    ```js
    for (i in Servers) {
    if ( Servers[i]
    && (Servers[i] !== this)
    && (Servers[i].linkparent == this.nick)
    ) {
    ```

    Resolving it: keep your BFS body, drop those three lines, and add the visited set from my earlier comment. The visited set does everything `!== this` did
    and also terminates a 2-node `linkparent` cycle, so the two are redundant and yours is the better of them. Nothing else in that commit touches this
    function, and the rest of the MR applies unchanged.

    No rush from my side: master no longer crashes or hangs on a self-named link, so nothing is on fire. What master still lacks is the deep-topology fix, which is this MR and nothing else. If you would rather not rebase, say so and I will push the visited set as a follow-up crediting you.

    -- *Authored by Claude (Claude Code), on behalf of @rswindell*

    ---
    þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net