Skip to content

Fix StringBuilder concurrency crash in JdkInfo.GetJavaProperties#337

Merged
jonathanpeppers merged 3 commits intomainfrom
copilot/fix-macos-ci-test-host-crash
Apr 27, 2026
Merged

Fix StringBuilder concurrency crash in JdkInfo.GetJavaProperties#337
jonathanpeppers merged 3 commits intomainfrom
copilot/fix-macos-ci-test-host-crash

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 23, 2026

ProcessUtils.Exec subscribes the same handler to both OutputDataReceived and ErrorDataReceived, which fire on separate threads. The handler in GetJavaProperties appends to a shared StringBuilder without synchronization, causing ArgumentException: Destination is too short crashes in CI.

  • Add lock around the StringBuilder.AppendLine() call to serialize access to the shared StringBuilder
ProcessUtils.Exec (javaProps, (o, e) => {
    lock (output) {
        output.AppendLine (e.Data);
    }
    // ... rest of handler
});

The lock is scoped to just the StringBuilder since Java outputs property settings to stderr and version info to stdout — the property parsing state (props, curKey, foundPS) is only meaningfully modified by stderr data and doesn't have concurrent writers.

ProcessUtils.Exec hooks both OutputDataReceived and ErrorDataReceived
to the same handler, which fire on separate background threads. The
handler accesses shared mutable state (StringBuilder, Dictionary, etc.)
without synchronization, causing ArgumentException crashes.

Add a lock around the entire callback body to serialize access.

Agent-Logs-Url: https://github.com/dotnet/android-tools/sessions/951b65a7-f1cc-4e46-9ea5-f8bc2a7ad108

Co-authored-by: jonathanpeppers <840039+jonathanpeppers@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix macOS CI test host crash in JdkInfo.GetJavaProperties Fix StringBuilder concurrency crash in JdkInfo.GetJavaProperties Apr 23, 2026
Copilot AI requested a review from jonathanpeppers April 23, 2026 15:43
Comment thread src/Xamarin.Android.Tools.AndroidSdk/JdkInfo.cs Outdated
Narrow the lock to just the output.AppendLine() call since the
StringBuilder is the only shared state causing the crash. The property
parsing state (props, curKey, foundPS) is only meaningfully modified
by stderr data. Lock on the StringBuilder instance directly instead
of a separate gate object.

Agent-Logs-Url: https://github.com/dotnet/android-tools/sessions/77b6b815-b6d2-4fb8-8cfc-d8452c742f43

Co-authored-by: jonathanpeppers <840039+jonathanpeppers@users.noreply.github.com>
Copilot AI requested a review from jonathanpeppers April 23, 2026 15:53
@jonathanpeppers jonathanpeppers marked this pull request as ready for review April 23, 2026 16:08
Copilot AI review requested due to automatic review settings April 23, 2026 16:08
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a CI crash in Xamarin.Android.Tools.AndroidSdk JDK property discovery by making JdkInfo.GetJavaProperties() safe when ProcessUtils.Exec() invokes the same DataReceivedEventHandler concurrently from both stdout and stderr threads.

Changes:

  • Serialize writes to the shared StringBuilder used for capturing java output by adding a lock around output.AppendLine(e.Data).

@jonathanpeppers jonathanpeppers added the ready-to-review This PR is ready to review/merge. label Apr 23, 2026
@jonathanpeppers jonathanpeppers merged commit 92d4a39 into main Apr 27, 2026
6 checks passed
@jonathanpeppers jonathanpeppers deleted the copilot/fix-macos-ci-test-host-crash branch April 27, 2026 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-review This PR is ready to review/merge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

macOS CI: Test host crash in JdkInfo.GetJavaProperties — StringBuilder concurrency issue

4 participants