aosp: wrap alarm() and make it use a thread-directed timer#11340
Open
rogerzanoni wants to merge 1 commit into
Open
aosp: wrap alarm() and make it use a thread-directed timer#11340rogerzanoni wants to merge 1 commit into
rogerzanoni wants to merge 1 commit into
Conversation
Contributor
🤖 Gemini Suggested Commit Message💡 Pro Tips for a Better Commit Message:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a thread-directed alarm() wrapper (__abi_wrap_alarm) for Android in modular Starboard to ensure reliable delivery of SIGALRM signals using thread-local POSIX timers. Feedback on the implementation suggests optimizing the case where seconds is 0 by bypassing timer creation, simplifying the gettid() retrieval since it is always available on Android, and switching from CLOCK_MONOTONIC to CLOCK_REALTIME to align with standard POSIX alarm() behavior during device suspend.
| // A one-shot POSIX timer used to implement a thread-directed alarm() | ||
| struct ThreadAlarmTimer { | ||
| timer_t timer = nullptr; | ||
| bool created = false; |
Contributor
There was a problem hiding this comment.
Is this necessary? Can we just check if timer is not nullptr instead?
Contributor
|
@sacuff @jellefoks ptal |
sacuff
reviewed
Jul 22, 2026
| new_value.it_value.tv_sec = seconds; | ||
| struct itimerspec old_value = {}; | ||
| if (timer_settime(thread_timer.timer, 0, &new_value, &old_value) != 0) { | ||
| // fallback |
| // A one-shot POSIX timer used to implement a thread-directed alarm() | ||
| struct ThreadAlarmTimer { | ||
| timer_t timer = nullptr; | ||
| bool created = false; |
Bionic's implementation of alarm triggers a setitimer set with ITIMER_REAL
making it a process-directed signal.
From the signal(7) manpage:
A process-directed signal may be delivered to any one of the threads
that does not currently have the signal blocked. If more than one of the
threads has the signal unblocked, then the kernel chooses an arbitrary
thread to which to deliver the signal.
Android's ART runtime/runtime.cc configures only a small set of signals:
void Runtime::BlockSignals() {
SignalSet signals;
signals.Add(SIGPIPE);
signals.Add(SIGQUIT);
signals.Add(SIGUSR1);
signals.Block();
}
SIGALRM is unblocked on the ART's main thread and it may catch the
process-directed signal, never letting it reach cobalt/nplb calling threads,
making it unreliable to wait for the signal using musl's implementation.
Fixes PosixClockNanosleepTest.ErrorEintrAbsoluteSleep,
PosixClockNanosleepTest.ErrorEintrRelativeSleep and
PosixNanosleepTests.ErrorEintr which were failing with a "signal never
interrupted sleep" error.
Bug: 532068409
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
3 participants
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.
Bionic's implementation of alarm triggers a setitimer set with ITIMER_REAL making it a process-directed signal.
From the signal(7) manpage:
A process-directed signal may be delivered to any one of the threads
that does not currently have the signal blocked. If more than one of the
threads has the signal unblocked, then the kernel chooses an arbitrary
thread to which to deliver the signal.
Android's ART runtime/runtime.cc configures only a small set of signals:
void Runtime::BlockSignals() {
SignalSet signals;
signals.Add(SIGPIPE);
signals.Add(SIGQUIT);
signals.Add(SIGUSR1);
signals.Block();
}
SIGALRM is unblocked on the ART's main thread and it may catch the process-directed signal, never letting it reach cobalt/nplb calling threads, making it unreliable to wait for the signal using musl's implementation.
Fixes PosixClockNanosleepTest.ErrorEintrAbsoluteSleep, PosixClockNanosleepTest.ErrorEintrRelativeSleep and PosixNanosleepTests.ErrorEintr which were failing with a "signal never interrupted sleep" error.
Bug: 532068409