fix(semaphore): use absolute deadline for POSIX TimedWait#17
Open
Rakdos8 wants to merge 1 commit into
Open
Conversation
sem_timedwait takes an absolute CLOCK_REALTIME deadline, but the code passed a relative duration, placing the deadline near the epoch so the wait always timed out instantly without blocking on Linux/Android. Compute the deadline from clock_gettime(CLOCK_REALTIME). Add tests covering timeout, already-signaled and signaled-before-timeout cases.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
CcpSemaphore::TimedWait(generic POSIX path) passed a relative duration tosem_timedwait, which expects anabsolute deadline against
CLOCK_REALTIME. The computedtimespec(tv_sec = timeoutMs/1000, i.e. a few secondsafter the epoch) is always far in the past, so the wait timed out immediately without ever blocking on Linux/Android —
TimedWaitreturnedfalseinstantly regardless of the requested timeout. The pre-existing testCcpSemaphore.DefaultConstructorCreatesNonSignaledSemaphore, which expects a ~1000 ms wait, was already failing onthe generic POSIX path because of this. macOS (mach semaphore) and Windows paths are not affected.
Fix
TimedWaitnow derives an absolute deadline fromclock_gettime(CLOCK_REALTIME), adds the requested timeout, andnormalizes the nanosecond overflow before calling
sem_timedwait. Added#include <time.h>.Tests
Added three tests in
tests/CcpSemaphore.cpp(GoogleTest):TimedWaitWaitsAndReturnsFalseOnTimeoutblocks ~500 msand returns
false(direct regression test for the relative/absolute confusion),TimedWaitReturnsTrueWhenAlreadySignaledreturnstrueimmediately, andTimedWaitReturnsTrueWhenSignaledBeforeTimeoutreturnstruewhen signaled from another thread before the deadline.Reviewed manually; not compiled locally as the vcpkg toolchain is unavailable in this environment.
Scope
Generic POSIX branch only (
#else). macOS and Windows code paths are unchanged. No public API change.