• src/sbbs3/ringbuf.c

    From rswindell@1:103/705 to CVS commit on Mon Aug 26 16:37:52 2019
    src/sbbs3 ringbuf.c 1.31 1.32
    Update of /cvsroot/sbbs/src/sbbs3
    In directory cvs:/tmp/cvs-serv30956

    Modified Files:
    ringbuf.c
    Log Message:
    Implement a simple bound-checker in RingBufWrite():
    if the ringbuf is shared among multiple threads (e.g. the sbbs->outbuf is shared between output_thread() and passthru_thread()) - it was possible
    for a race condition to occur between the caller would call RingBufFree
    to determine the available space in the ringbuf and the call to RingBufWrite which would happily overflow the allocated buffer if more data was
    written to the ringbuf (by another thread) in the unprotected time between the RingBufFree and RingBufWrite calls.

    Now, RingBufWrite() can perform short-writes and will return a length less
    than what was requested to write when there is not enough available space
    to write the requested length.

    Hopefully this resolves the corruption/crash issue Deuce is seeing in
    sbbs's passthru_thread().


    --- SBBSecho 3.09-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)
  • From Rob Swindell (in GitKraken)@1:103/705 to Git commit to main/sbbs/master on Fri Mar 24 13:27:09 2023
    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* signaledSince commit fafed094c52b0ca1 (2 months ago), the Synchronet Web Server onmy Windows systems has occasionally gone into a state where every HTTP sessionthread was causing 100% CPU utilization and each new HTTP session threadwould just exacerbate the problem eventually leading to complete systeminstability/unresponsiveness until the sbbs instance was terminated. This wasmore readily reproducible on a Win7-32 VM, but would occasionally, thoughmuch 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 stuckin 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 executedimmediately after the output ringbuf was created, but before writebuf() wasever called (which would have actually placed data in the output buffer),causing a potential high-utilization infinite loop: the data_event is signaledbut there is no data and the event is never reset and nothing can ever adddata to the ringbuf due to starvation of CPU cycles.Uses of ringbuf's data_event elsewhere in Synchronet don't seem to be subjectto this issue since they always call RingBufRead after, which will clear thedata_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 addedto RingBufInit(): the preexisting 'empty_event' was initialized with acorrect initiate state of 'signaled' (because by default a ringbuf is empty)but the newly added events (data_event and hi

    ghwater_event) should *not* beinitially-signaled because... the ringbuf is empty. So I added some parametercomments to these calls to CreateEvent() to hopefully make that more clearand prevent similar mistakes in the future.Relieved to have this one resolved finally.
    --- SBBSecho 3.20-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)