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