From 8d5e993169256ed80cc208d010179e9ff3f55708 Mon Sep 17 00:00:00 2001 From: Michael Lotz Date: Thu, 27 Aug 2009 02:15:20 +0000 Subject: [PATCH] mmlr+anevilyak: Unconditionally release the reader sem when the previous buffer size was 0 instead of testing for fParserWaiting as there is a race condition between setting it to true in the parser thread and checking for it in the reader thread so a release_sem() could be missed causing #4343. To reduce the possible side effect of needlessly looping through acquire_sem() for too many unconditionally released sems, we eat them in the parser thread once we're sure we can't miss an event because of that. Also added a TODO about a memcpy() optimization as pointed out by Rene. +alphabranch git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@32733 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- src/apps/terminal/TermParse.cpp | 14 ++++++++------ src/apps/terminal/TermParse.h | 1 - 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/apps/terminal/TermParse.cpp b/src/apps/terminal/TermParse.cpp index 0f8d10607f..2e7c50e957 100644 --- a/src/apps/terminal/TermParse.cpp +++ b/src/apps/terminal/TermParse.cpp @@ -74,7 +74,6 @@ TermParse::TermParse(int fd) fReadBufferSize(0), fParserBufferSize(0), fParserBufferOffset(0), - fParserWaiting(false), fBuffer(NULL), fQuitting(true) { @@ -252,7 +251,7 @@ TermParse::PtyReader() memcpy(fReadBuffer + readPos, buf, nread); bufferSize = atomic_add(&fReadBufferSize, nread); - if (bufferSize == 0 && fParserWaiting) + if (bufferSize == 0) release_sem(fReaderSem); bufferSize += nread; @@ -1012,16 +1011,17 @@ TermParse::_ReadParserBuffer() // wait for new input from pty if (fReadBufferSize == 0) { - fParserWaiting = true; - status_t status = B_OK; while (fReadBufferSize == 0 && status == B_OK) { do { status = acquire_sem(fReaderSem); } while (status == B_INTERRUPTED); - } - fParserWaiting = false; + // eat any sems that were released unconditionally + int32 semCount; + if (get_sem_count(fReaderSem, &semCount) == B_OK && semCount > 0) + acquire_sem_etc(fReaderSem, semCount, B_RELATIVE_TIMEOUT, 0); + } if (status < B_OK) { fBuffer->Lock(); @@ -1034,6 +1034,8 @@ TermParse::_ReadParserBuffer() toRead = ESC_PARSER_BUFFER_SIZE; for (int32 i = 0; i < toRead; i++) { + // TODO: This could be optimized using memcpy instead and + // calculating space left as in the PtyReader(). fParserBuffer[i] = fReadBuffer[fBufferPosition]; fBufferPosition = (fBufferPosition + 1) % READ_BUF_SIZE; } diff --git a/src/apps/terminal/TermParse.h b/src/apps/terminal/TermParse.h index 4e881ff8d7..5b7fa84070 100644 --- a/src/apps/terminal/TermParse.h +++ b/src/apps/terminal/TermParse.h @@ -93,7 +93,6 @@ private: uchar fParserBuffer[ESC_PARSER_BUFFER_SIZE]; int32 fParserBufferSize; int32 fParserBufferOffset; - volatile bool fParserWaiting; int fLockFlag;