https://gitlab.synchro.net/main/sbbs/-/commit/ffe092b65b38633a5ca185e3
Modified Files:
src/sbbs3/ringbuf.c
Log Message:
Set initial state of 'data_event' and 'highwater_event' to *not* signaled

Since commit fafed094c52b0ca1 (2 months ago), the Synchronet Web Server on
my Windows systems has occasionally gone into a state where every HTTP session thread was causing 100% CPU utilization and each new HTTP session thread
would just exacerbate the problem eventually leading to complete system instability/unresponsiveness until the sbbs instance was terminated. This was more readily reproducible on a Win7-32 VM, but would occasionally, though
much less frequently, happen in a native instance on Win10-64 as well.

Using the VisualStudio debugger, I was able to narrow it down to this:

Each new HTTP thread (eventually, hundreds of such threads) would get stuck
in this loop in http_output_thread():

while(session->socket!=INVALID_SOCKET) {

/* Wait for something to output in the RingBuffer */
if((avail=RingBufFull(obuf))==0) { /* empty */
if(WaitForEvent(obuf->data_event, 1000) != WAIT_OBJECT_0)
continue;
/* Check for spurious sem post... */
if((avail=RingBufFull(obuf))==0)
continue; // <--- data_event signaled, but never cleared
}

There appears to be a race condition where this logic could be executed immediately after the output ringbuf was created, but before writebuf() was ever called (which would have actually placed data in the output buffer), causing a potential high-utilization infinite loop: the data_event is signaled but there is no data and the event is never reset and nothing can ever add
data to the ringbuf due to starvation of CPU cycles.

Uses of ringbuf's data_event elsewhere in Synchronet don't seem to be subject to this issue since they always call RingBufRead after, which will clear the data_event when the ringbuf is actually empty (no similar loops to this one).

The root cause just appears to be a simple copy/paste issue in the code added to RingBufInit(): the preexisting 'empty_event' was initialized with a
correct initiate state of 'signaled' (because by default a ringbuf is empty) but the newly added events (data_event and highwater_event) should *not* be initially-signaled because... the ringbuf is empty. So I added some parameter comments to these calls to CreateEvent() to hopefully make that more clear
and prevent similar mistakes in the future.

Relieved to have this one resolved finally.

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

https://gitlab.synchro.net/main/sbbs/-/commit/284d2357304c84f2eafc5567
Modified Files:
src/sbbs3/ringbuf.c
Log Message:
Fix typos in comments

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

https://gitlab.synchro.net/main/sbbs/-/commit/9e7885d0f08b00641b058beb
Modified Files:
src/sbbs3/ringbuf.c
Log Message:
ringbuf: only signal events on state transitions in mutex-held builds

RingBufWrite called ResetEvent(empty_event) and SetEvent(data_event) unconditionally on every call, regardless of whether the ring's
empty/non-empty state actually changed. In sexyz, send_byte calls
RingBufWrite once per transmitted byte, so a download was paying for
2-3 event-object operations per byte. On Win32 those operations are
real kernel syscalls; on Unix they go through xpevent.c (pthread
mutex+cond) which is much cheaper.

A reporter on Windows saw 447 KB/s downloads on a link that delivered
~43 MB/s for the reverse direction (the upload path takes data through recv()->inbuf[] in 64 KiB chunks with no per-byte event signaling, so
it wasn't affected).

The unconditional signaling exists because RINGBUF_MUTEX is optional:
without the mutex you cannot safely check "is this a transition" and
then signal, since another writer/reader could change state between
the check and the call. But every Synchronet build that defines
RINGBUF_EVENT also defines RINGBUF_MUTEX (sbbs.vcxproj, websrvr.vcxproj, sbbsexec.vcxproj, sexyz.vcxproj, sbbsdefs.mk, CMakeLists.txt, and the
sbbs.h fallback), so the slow path is the only one in practice.

Add a #ifdef RINGBUF_MUTEX fast path inside the #ifdef RINGBUF_EVENT
block in RingBufWrite and RingBufRead that uses the pre-update fill
level (already captured for the truncation check) together with the
post-clamp byte count to signal only on actual empty<->non-empty and below<->at-or-above-highwater transitions. The mutexless #else path
preserves the existing always-signal semantics verbatim.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

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

https://gitlab.synchro.net/main/sbbs/-/commit/d898b01aeacc032b0471fdd2
Modified Files:
src/sbbs3/ringbuf.c
Log Message:
ringbuf: fix RingBufReInit not clearing highwater_event

After pHead=pTail=pStart the ring is empty (fill==0), so the
highwater_event should always be in the reset state on return. The
existing guard tested `RINGBUF_FILL_LEVEL(rb) >= rb->highwater_mark`,
which is never true at this point, so the ResetEvent was never called
and a previously-set highwater_event survived the reinit. Whoever next
waited on it then took the "highwater reached" path against an empty
ring.

Drop the unsatisfiable fill check; keep only the "event configured"
guards so the call is a no-op when events or a highwater mark aren't
in use.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

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