[NativeAOT] Use POSIX primitives for GC bridge processing by simonrozsival · Pull Request #12141 · dotnet/android

simonrozsival · 2026-07-16T23:21:49Z

Summary

Replace the NativeAOT-reachable GC bridge's C++ threading primitives with POSIX primitives.

This removes the std::thread, std::binary_semaphore, and associated libc++ atomic-wait/thread runtime dependency from this subsystem as part of #12139.

Changes

replace the detached std::thread worker with pthread_create() / pthread_detach();
replace std::binary_semaphore with sem_t;
retry sem_wait() after EINTR;
preserve acquire/release synchronization for the shared callback argument with compiler atomics;
surface pthread/semaphore failures through the existing abort helpers;
document the JavaMarshal one-outstanding-round invariant that makes shared_args an intentional single-slot handoff rather than a queue.

The bridge-processing algorithm and callback lifecycle are unchanged.

Validation

git diff --check;
Release build of src/native/native-nativeaot.csproj;
Release build of src/native/native-clr.csproj;
focused concurrency review covering initialization, the one-outstanding-round contract, memory ordering, EINTR, and detached worker behavior;
latest head: 12bc25740.

Part of #12139.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 187b207a-083b-461e-9071-e9aab61c9d07

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 187b207a-083b-461e-9071-e9aab61c9d07

Clarify that JavaMarshal permits only one outstanding bridge round, so shared_args is intentionally a single-slot handoff. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70f63eb7-6599-414c-a947-d860705aa0fa

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

simonrozsival · 2026-07-17T20:13:38Z

/review

github-actions · 2026-07-17T20:14:23Z

✅ Android PR Reviewer completed successfully!

github-actions

🤖 Code Review — ✅ LGTM (with minor suggestions)

Verdict: ✅ LGTM · Issues: 0 ❌ · 0 ⚠️ · 2 💡

This is a clean, well-scoped refactor that swaps the GC bridge's std::thread / std::binary_semaphore / std::atomic primitives for POSIX pthread + sem_t + compiler __atomic builtins, as part of the libc++-removal effort (#12139).

What was verified

Correctness of the handoff: publish_shared_args (RELEASE store + sem_post) / wait_for_shared_args (sem_wait + ACQUIRE load) preserve the acquire/release ordering of the original std::atomic. The single-slot design is safe given the documented JavaMarshal "one outstanding round" invariant.
EINTR handling: sem_wait is correctly retried in a loop on EINTR (native-rules Postmortem #22). 👍
Consistency: pthread and __atomic_* builtins are already established conventions in this codebase (assembly-store.cc, os-bridge.cc, pinvoke-override), so the approach fits in well.
Error handling: pthread/semaphore failures are surfaced through the existing abort_unless helpers, consistent with the rest of the host.
Include cleanup: removed <atomic>, <thread>, <semaphore>, <shared_mutex>, <unordered_map> — none appear to be used elsewhere in the header. 👍
__atomic_* are compiler builtins (not user identifiers), so the reserved-double-underscore rule doesn't apply here.

Suggestions (non-blocking, posted inline)

💡 Match assembly-store.cc's single-step detached-thread creation and consider naming the worker thread for diagnostics.
💡 A brief note that the semaphore (not the atomics) is the real synchronization point would aid future readers.

Nice, tightly-focused change with a helpful invariant comment. CI shows no required checks failing at review time.

Generated by Android PR Reviewer for #12141 · 84.2 AIC · ⌖ 13.1 AIC · ⊞ 6.8K
_{Comment /review to run again}

github-actions · 2026-07-17T20:18:50Z

+void GCBridge::start_bridge_processing_thread () noexcept
+{
+	pthread_t thread {};
+	int ret = pthread_create (&thread, nullptr, bridge_processing_thread_entry, nullptr);


🤖 💡 Native C++ — Consider creating the worker already-detached via pthread_attr_setdetachstate (&attr, PTHREAD_CREATE_DETACHED) before pthread_create, matching the sibling pattern in assembly-store.cc (start_writer_locked). The current create-then-pthread_detach sequence works, but a single-step detached creation is consistent with the rest of the CLR host and avoids a separate failure/abort path.

While here, pthread_setname_np (thread, "gc-bridge") would make this worker identifiable in logcat/tombstones and traces — helpful given it runs the GC bridge on every collection.

(Rule: Consistency with repo native patterns)

github-actions · 2026-07-17T20:18:50Z

+{
+	__atomic_store_n (&shared_args, args, __ATOMIC_RELEASE);
+
+	int ret = sem_post (&shared_args_semaphore);


🤖 💡 Native C++ — sem_post/sem_wait already establish a happens-before relationship, so the explicit __ATOMIC_RELEASE/__ATOMIC_ACQUIRE on shared_args are technically redundant for this single-slot handoff (which the header comment nicely documents). Keeping them as belt-and-suspenders is fine, but a one-line note here that the semaphore is the real synchronization point — and the atomics just make the intent explicit — would prevent a future reader from assuming lock-free concurrent access is intended.

(Rule: Documentation of concurrency intent)

## Summary Continue the printf-style logging migration introduced by #12140 in the shared native timing infrastructure. The timing headers are used by MonoVM, CoreCLR, and NativeAOT. All three runtimes now use the same printf-style path for variable timing diagnostics rather than maintaining MonoVM-only `std::format` branches. **Base/dependency:** #12140 must merge first. This PR targets `dev/simonrozsival/nativeaot-printf-logging` so its diff contains only the timing migration. Part of #12139. ## Scope Only two shared timing headers change: - `src/native/common/include/runtime-base/timing.hh` - `src/native/common/include/runtime-base/timing-internal.hh` These files do not overlap #12141, #12142, #12145, #12148, #12150, or #12155. ## Changes ### Managed timing records - replace the owning `std::format` result in `Timing::do_log()` with `log_writef()` for every runtime; - preserve the exact `message; elapsed: seconds:milliseconds::nanoseconds` field order; - pass duration values as explicitly converted `unsigned long long` values matching `%llu`. ### Fast timing diagnostics Migrate variable diagnostics to `log_warnf()` for every runtime: - timing event buffer reallocation sizes; - `CLOCK_MONOTONIC_RAW` errors; - unknown event-kind values; - invalid event-index source method names. Constant warning messages continue using the existing non-formatting `string_view` overload. ## Behavior preserved - timing enablement and category gating are unchanged; - timing sequence acquisition/release is unchanged; - elapsed duration units, values, and output order are unchanged; - event-buffer growth, clock error handling, unknown-event fallback, and index validation are unchanged; - no timing data structures, locks, vectors, strings, or event lifecycle code change. ## Runtime behavior | Runtime | Result | |---|---| | NativeAOT | Uses #12140 printf helpers for variable timing diagnostics | | CoreCLR | Uses #12140 printf helpers for variable timing diagnostics | | MonoVM | Uses the matching MonoVM printf helper implementation from #12140 | ## Measured impact Representative Android arm64 Release objects compiled before the MonoVM path was unified: | Runtime/object | Before | After | Difference | |---|---:|---:|---:| | CoreCLR `internal-pinvokes-clr.cc.o` | 145,472 B | 17,080 B | -128,392 B (-88.26%) | | NativeAOT `host.cc.o` | 176,104 B | 175,960 B | -144 B | CoreCLR's representative object drops from 58 formatting symbols to zero. NativeAOT's representative object still contains formatting symbols from other included functionality, but the timing call sites migrated here no longer instantiate them. ## Non-goals - no new timing tests or logging test framework; - no change to timing data ownership or synchronization; - no changes to the logging helper implementation introduced by #12140; - no attempt to migrate unrelated timing output/file-generation code outside these shared timing headers. ## Validation - `git diff --check`; - NDK Clang C++23 syntax compilation of all 36 Android arm64 Release NativeAOT source variants; - NDK Clang C++23 syntax compilation of all 38 Android arm64 Release CoreCLR source variants; - NDK Clang C++23 syntax compilation of all 31 Android arm64 Release MonoVM source variants; - representative before/after object compilation, size comparison, and symbol inspection; - focused review of chrono values, integer format widths, source-location output, and category semantics; - latest head: `21ff66b26`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

## Summary Continue the printf-style logging migration introduced by #12140 by converting shared CoreCLR/NativeAOT JNI reference logging in `OSBridge`. The `src/native/clr/` location is shared infrastructure: the NativeAOT host explicitly compiles `clr/host/os-bridge.cc`, while MonoVM uses its separate `mono/monodroid/osbridge.cc` implementation. This PR therefore affects CoreCLR and NativeAOT, but not MonoVM. **Base/dependency:** #12140 must merge first. This PR targets `dev/simonrozsival/nativeaot-printf-logging` so its diff contains only this migration. Part of #12139. ## Scope Only two files change: - `src/native/clr/host/os-bridge.cc` - `src/native/clr/include/host/os-bridge.hh` This does not overlap the open sibling work: - #12141 changes `gc-bridge.cc/.hh` — worker-thread and semaphore handoff; - #12142 changes logging/path ownership and source-location parsing elsewhere; - #12145 changes `bridge-processing.cc/.hh` — temporary-peer graph storage; - #12148 changes shared host/runtime utility logging. ## Changes ### Format reference records once - add a private printf-checked `OSBridge::log_itf()` helper; - use `vasprintf()` to produce one NUL-terminated reference-log line; - pass the same formatted line to the existing logcat and file-writing path; - release the C allocation after both destinations have consumed it; - on allocation failure, fall back to the static format string rather than throwing from a `noexcept` `std::format` path. ### Migrate JNI reference events Replace six owning `std::format` constructions: - new global reference; - deleted global reference; - new weak global reference; - deleted weak global reference; - new local reference; - deleted local reference. The replacements use `%d`, `%p`, `%c`, and `%s` with compiler format checking from the private helper declaration. ### Remove ranges-heavy stack-trace splitting - replace `std::views::split` with an in-place `std::string_view` newline scan; - preserve empty, consecutive, and trailing lines; - preserve logcat-only, file-only, and combined output behavior; - write non-NUL-terminated line views with explicit lengths; - convert stack-trace and raw gref logcat forwarding to `%.*s` / `%s`. ## Responsibility boundary with #12141 - `GCBridge` (#12141) controls **when and where** a bridge round runs: callback handoff, worker thread, semaphore synchronization, and Java GC triggering. - `OSBridge` (this PR) tracks and logs **JNI reference events**: gref/weak-gref counters, reference creation/deletion lines, and optional stack traces. The classes call each other through existing stable helpers, but this PR changes no GC scheduling, graph processing, callback lifecycle, or synchronization state. ## Behavior preserved - reference counters are incremented/decremented at the same points; - logging remains gated by `LOG_GREF` / `LOG_LREF` before formatting; - each reference line is still emitted to logcat and, when configured, the corresponding file; - stack traces retain one output line per newline-delimited segment, including empty segments; - logcat/file flushing behavior is unchanged; - pointer, reference-type, thread-name, thread-id, and counter fields retain the same ordering and textual structure. ## Measured impact Android arm64 Release NativeAOT `os-bridge.cc` object, compiled with the repository-generated production NDK command: | Artifact | Before | After | Difference | |---|---:|---:|---:| | `os-bridge.cc.o` | 168,784 B | 31,288 B | -137,496 B (-81.46%) | | format/basic-string symbols | 21 | 6 | -15 | The remaining C++ string symbols come from other shared header functionality; the six `std::format` reference-log instantiations are removed. ## Non-goals - no new logging test framework is introduced; - no GC bridge synchronization or graph-processing code changes; - no MonoVM `osbridge.cc` changes; - no changes to the public logging helpers introduced by #12140. ## Validation - `git diff --check`; - no file overlap with #12141, #12142, #12145, or #12148; - NDK Clang C++23 syntax compilation of all 36 Android arm64 Release NativeAOT source variants; - NDK Clang C++23 syntax compilation of all 38 Android arm64 Release CoreCLR source variants; - before/after NativeAOT object compilation and symbol inspection; - focused read-only review of newline handling, allocation cleanup, OOM fallback, format types, category gating, and dual-destination output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

## Summary Continue the printf-style logging migration introduced by #12140 for native configuration and parsing diagnostics. This PR converts CoreCLR/NativeAOT host-environment messages and the shared MonoVM/CoreCLR/NativeAOT integer-parsing messages to the printf helpers from #12140. Shared code now has one logging path instead of runtime-specific format branches. **Base/dependency:** #12140 must merge first. This PR targets `dev/simonrozsival/nativeaot-printf-logging` so its diff contains only this migration. Part of #12139. ## Scope Only two files change: - `src/native/clr/include/host/host-environment.hh` - `src/native/common/include/runtime-base/strings.hh` These files do not overlap #12141, #12142, #12145, #12148, #12150, or #12153. ## Changes ### Host environment diagnostics `host-environment.hh` is shared by CoreCLR and NativeAOT, but not MonoVM. - migrate generated system-property diagnostics to `%s` formatting; - migrate XDG directory creation diagnostics to `%s` formatting; - migrate XDG directory failure diagnostics while preserving the original errno text; - retain `optional_string()` handling for null C-string inputs. ### Integer parsing diagnostics `strings.hh` is shared by all three runtimes. Its variable diagnostics now use `log_errorf()` without runtime-specific branches: - an invalid starting index, using `%zu` for `size_t`; - signed/unsigned range failures, using explicitly converted `%lld` / `%llu` values; - values that do not represent an integer in the selected `%d` base; - trailing non-numeric characters. The header forward-declares only `log_errorf()` instead of including the heavyweight formatting declarations from `shared/log_types.hh`. ## Behavior preserved - integer conversion logic, range checks, errno handling, and output assignment are unchanged; - the copied parse buffer remains NUL-terminated before `%s` logging; - host environment variables and XDG directory creation behavior are unchanged; - debug category gating and unconditional error/warning semantics remain unchanged. ## Runtime behavior | Runtime | Result | |---|---| | NativeAOT | Uses #12140 printf helpers for all migrated diagnostics | | CoreCLR | Uses #12140 printf helpers for shared parsing and host-environment diagnostics | | MonoVM | Uses the matching MonoVM `log_errorf()` implementation for shared parsing diagnostics | ## Measured impact Representative Android arm64 Release objects compiled before the MonoVM path was unified: | Runtime/object | Before | After | Difference | |---|---:|---:|---:| | NativeAOT `host-environment.cc.o` | 131,288 B | 130,848 B | -440 B | | CoreCLR `timing-internal.cc.o` | 194,048 B | 193,088 B | -960 B | These call sites did not contain explicit `std::format` expressions, so the impact is smaller than #12150/#12153; the change prevents their formatted logging templates from being instantiated. ## Non-goals - no new configuration or parsing behavior; - no changes to environment-variable storage, XDG path construction, or integer parsing buffers; - no new logging tests; - no attempt to migrate unrelated configuration code outside these two headers. ## Validation - `git diff --check`; - NDK Clang C++23 syntax compilation of all 36 Android arm64 Release NativeAOT source variants; - NDK Clang C++23 syntax compilation of all 38 Android arm64 Release CoreCLR source variants; - NDK Clang C++23 syntax compilation of all 31 Android arm64 Release MonoVM source variants; - representative before/after object compilation and size comparison; - focused review of signed/unsigned conversions, format widths, null termination, include dependencies, and runtime behavior; - latest head: `72cad5d01`. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>

[NativeAOT] Use POSIX primitives for GC bridge processing

9e579ea

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 187b207a-083b-461e-9071-e9aab61c9d07

Copilot AI review requested due to automatic review settings July 16, 2026 23:21

simonrozsival requested a review from jonathanpeppers as a code owner July 16, 2026 23:21

simonrozsival added the drop-libcpp Work to remove the libc++ dependency from Android NativeAOT label Jul 16, 2026

Copilot started reviewing on behalf of simonrozsival July 16, 2026 23:22 View session

Copilot AI reviewed Jul 16, 2026

simonrozsival added 4 commits July 17, 2026 08:41

[NativeAOT] Encapsulate POSIX GC bridge operations

8d263b1

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 187b207a-083b-461e-9071-e9aab61c9d07

[NativeAOT] Make POSIX helpers private GCBridge methods

a8d4e86

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 187b207a-083b-461e-9071-e9aab61c9d07

[NativeAOT] Keep GC bridge callback inline

58dd1b6

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 187b207a-083b-461e-9071-e9aab61c9d07

jonathanpeppers requested a review from Copilot July 17, 2026 13:23

Copilot started reviewing on behalf of jonathanpeppers July 17, 2026 13:23 View session

Copilot AI reviewed Jul 17, 2026

View reviewed changes

Merge branch 'main' into dev/simonrozsival/nativeaot-posix-gc-bridge

98fcba6

simonrozsival added the ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable). label Jul 17, 2026

github-actions Bot reviewed Jul 17, 2026

View reviewed changes

simonrozsival enabled auto-merge (squash) July 17, 2026 20:19

jonathanpeppers approved these changes Jul 20, 2026

View reviewed changes

simonrozsival merged commit 022b05c into main Jul 20, 2026
44 checks passed

simonrozsival deleted the dev/simonrozsival/nativeaot-posix-gc-bridge branch July 20, 2026 13:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[NativeAOT] Use POSIX primitives for GC bridge processing#12141

[NativeAOT] Use POSIX primitives for GC bridge processing#12141
simonrozsival merged 6 commits into
mainfrom
dev/simonrozsival/nativeaot-posix-gc-bridge

simonrozsival commented Jul 16, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

simonrozsival commented Jul 17, 2026

Uh oh!

github-actions Bot commented Jul 17, 2026 •

edited

Loading

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot Jul 17, 2026

Uh oh!

github-actions Bot Jul 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

simonrozsival commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Validation

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

simonrozsival commented Jul 17, 2026

Uh oh!

github-actions Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

🤖 Code Review — ✅ LGTM (with minor suggestions)

What was verified

Suggestions (non-blocking, posted inline)

Uh oh!

github-actions Bot Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

simonrozsival commented Jul 16, 2026 •

edited

Loading

github-actions Bot commented Jul 17, 2026 •

edited

Loading