Skip to content

feat: promote/demote a thread other than the current one on every platform - #46

Draft
roderickvd wants to merge 9 commits into
mozilla:masterfrom
roderickvd:generic-remote-thread-promotion
Draft

feat: promote/demote a thread other than the current one on every platform#46
roderickvd wants to merge 9 commits into
mozilla:masterfrom
roderickvd:generic-remote-thread-promotion

Conversation

@roderickvd

Copy link
Copy Markdown

This PR extends get_current_thread_info/promote_thread_to_real_time/demote_thread_from_real_time (and their C API and serialization counterparts) from Linux-only to macOS, iOS, Windows and Android. This is useful because RT promotion itself is not RT-safe, so we don't want to do that on the audio thread.

  • macOS/iOS: resolves a (pid, thread_id) pair to a Mach thread port via task_threads()/thread_info(), using task_for_pid() for threads in another process.
  • Android: setpriority(PRIO_PROCESS, tid, ...) targets an arbitrary thread directly.
  • Windows: promotes via OpenThread/SetThreadPriority instead of the MMCSS avrt.dll API, which only ever acts on the calling thread.

Also restores exact previous priority when demoting a remote thread.

Didn't touch the Linux parts, pending #45. Let me know if I should later rebase on that one.

Note: this is Claude-assisted handiwork from the camping. I only have my Mac with me to test.

@padenot

padenot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

This PR extends get_current_thread_info/promote_thread_to_real_time/demote_thread_from_real_time (and their C API and serialization counterparts) from Linux-only to macOS, iOS, Windows and Android. This is useful because RT promotion itself is not RT-safe, so we don't want to do that on the audio thread.

Interesting. When I'm promoting or demoting a thread, we've not started our real-time workload in Firefox, so it doesn't matter for us, do you have different requirements?

@padenot
padenot self-requested a review July 30, 2026 09:13
@roderickvd

Copy link
Copy Markdown
Author

That’s true for most of our cases as well. The (only) place where this happens in cpal is the with audio servers like PipeWire that can change their buffer sizes mid-stream when their graph changes. To re-request promotion with the new buffer sizes, we hand off to a background thread that needs to know the audio thread ID.

@padenot

padenot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

I see, thanks. The other one has been merged, this needs a hopefully light rebase and I'll look.

…tform

Extends get_current_thread_info/promote_thread_to_real_time/
demote_thread_from_real_time (and their C API and serialization
counterparts) from Linux-only to macOS, iOS, Windows and Android.

- macOS/iOS: resolves a (pid, thread_id) pair to a Mach thread port via
  task_threads()/thread_info(), using task_for_pid() for threads in
  another process.
- Android: setpriority(PRIO_PROCESS, tid, ...) targets an arbitrary
  thread directly.
- Windows: promotes via OpenThread/SetThreadPriority instead of the
  MMCSS avrt.dll API, which only ever acts on the calling thread.
On Android and Windows, demoting a thread promoted via
promote_thread_to_real_time reset it to a generic default
(niceness 0 / THREAD_PRIORITY_NORMAL) instead of its actual prior
priority. RtPriorityThreadInfo now carries the priority captured at
get_current_thread_info time, and demote_thread_from_real_time
restores it exactly, matching the existing macOS behavior.
The handle returned by promote_thread_to_real_time must not be passed
to demote_current_thread_from_real_time; demotion goes through
demote_thread_from_real_time(thread_info) instead, since promotion and
demotion may happen on different threads or processes.
Same clarification as the Rust API: atp_demote_thread_from_real_time
takes the thread_info, not the handle atp_promote_thread_to_real_time
returned. That handle still needs freeing separately via
atp_free_handle.
A plain #[cfg(target_os = "linux")] attribute does the same job as the
single-armed cfg_if! block.
A failing assertion previously left the spawned thread parked on
done_rx.recv() forever, potentially still at real-time priority. A
Drop guard now signals and joins it on every exit path, including
during unwinding.
A padding gap between pid and thread_id (the latter needs 8-byte
alignment) meant transmute-based (de)serialization could read
uninitialized bytes. Pack/unpack fields explicitly instead, matching
the other platform backends.

Also stop querying the remaining threads via thread_info once a match
is found in resolve_thread_port, just releasing their ports instead.
Thread ids are reused once a thread exits, so a tid captured in
RtPriorityThreadInfoInternal could, by the time promote/demote runs,
refer to an unrelated thread that was later assigned the same id.

Capture each thread's creation/start time alongside its tid (via
GetThreadTimes on Windows, /proc/<pid>/task/<tid>/stat on Android) and
re-check it before changing priority, failing with an explicit error
on a mismatch instead of silently reprioritizing the wrong thread.

Also switch both platforms' serialize/deserialize away from transmute
to explicit field packing, needed for the new field and consistent
with the mach backend, and narrow get_thread_priority on Windows to
only request THREAD_QUERY_INFORMATION.
ATP_THREAD_INFO_SIZE is a runtime extern size_t, not a compile-time
constant, so it can't portably size a stack array (MSVC rejects the
VLA GCC/Clang would allow). Heap-allocate via std::vector instead.

Also stop comparing the two atp_thread_info structs with a raw
memcmp: struct padding isn't guaranteed identical between two
independently-constructed instances even when every field matches.
Compare via a second serialization round-trip instead, since
atp_serialize_thread_info packs fields explicitly with no padding.
@roderickvd
roderickvd marked this pull request as draft July 31, 2026 14:24
@roderickvd
roderickvd force-pushed the generic-remote-thread-promotion branch from bc152bc to 999a30c Compare July 31, 2026 14:26
@roderickvd

Copy link
Copy Markdown
Author

I rebased it and put it some fixes. Same time, I converted this PR to draft because I don't feel confident without having tested this on various platforms first.

@padenot

padenot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

I've just clicked the thing to run our CI. It's not amazingly exhaustive, but it's better than nothing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants