Skip to content

[CI Visibility] Fix message loss and abandoned mutexes in the IPC channel - #9335

Open
tonyredondo wants to merge 4 commits into
masterfrom
tony/ci-visibility-ipc-cross-runtime-lock
Open

[CI Visibility] Fix message loss and abandoned mutexes in the IPC channel#9335
tonyredondo wants to merge 4 commits into
masterfrom
tony/ci-visibility-ipc-cross-runtime-lock

Conversation

@tonyredondo

@tonyredondo tonyredondo commented Sep 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary of changes

Fixes four bugs in the CI Visibility IPC channel (CircularChannel) that lose messages or leave the channel locked:

  • Opening a channel reset its read and write pointers, so every new IPC client dropped the messages the other side hadn't read yet.
  • A completely full buffer was marked with a virtual write position that overflows the ushort pointer with the default 64 KB buffer. The reader then re-read the same messages forever while holding the lock, until the process ran out of memory.
  • The constructor and the writer let AbandonedMutexException escape. An abandoned wait still transfers ownership, so the thread kept the mutex and every other process using the channel timed out. The reader already handled this ([Test Optimization] Fix MSTest 4.4.0 discovery and native retries #9192).
  • Disposing a reader dropped whatever was written during its last polling interval.

Reason for change

Coverage results, session tags and the executed test count in #9486 all go through this channel, and each of these bugs can lose them. The abandoned-mutex one is the TODO that #9317 left in CheckBuildLogsForErrors.

Implementation details

  • The constructor only initializes the file (size and pointers) when it's new or has an unexpected size.
  • The writer always leaves one byte free, so the write position never reaches the read position and the virtual position is never written. Reader and writer still understand a virtual position written by an older version.
  • The constructor, writer and reader share TryAcquireMutex, which treats an abandoned mutex as acquired and logs a warning. Callers release it as usual. The CheckBuildLogsForErrors TODO and its ignore patterns for these errors are removed. The pattern for ReleaseMutex failing after a successful wait stays, with a new comment: that one comes from the .NET 11 RC runtime bug below, and it shows up on Linux ARM64 when a .NET 8-10 test host shares a channel with the .NET 11 VSTest runner.
  • The reader waits on its stop event instead of sleeping, so Dispose wakes it right away, and it polls once more before it stops. Dispose already joins the polling thread before the channel releases the memory mapped file.

This PR doesn't fix the .NET 11 RC named mutex incompatibility with .NET 8-10 on Unix (dotnet/runtime#134491). That's fixed in the runtime for .NET 11 GA (dotnet/runtime#134595), so the existing .NET 11 test skips stay until we move to GA. An earlier version of this PR replaced the named mutex with a file lock on Unix; I dropped it so we don't maintain a separate lock for a runtime bug that's going away.

Test coverage

New CircularChannelTests, each checked locally to fail without its fix:

  • OpeningAnExistingChannelKeepsUnreadMessages
  • BufferNeverFillsCompletelyWithTheDefaultSize: with the default buffer size, a message that would fill the buffer exactly is rejected, and every message is delivered exactly once. The unit tests used an 8 KB buffer, which never overflows.
  • OpeningAChannelAndWritingAcquireAnAbandonedMutex
  • DisposingAReaderReadsMessagesWrittenBeforeIt

CircularChannelWriteTest now expects one byte to stay free. Datadog.Trace.Tests.Ci.Ipc passes locally on Linux with net8.0.

Other details

Related: #9317, #9327, dotnet/runtime#134491, dotnet/runtime#134595. #9486 is stacked on this PR.

@tonyredondo
tonyredondo force-pushed the tony/ci-visibility-ipc-cross-runtime-lock branch from 6ff2985 to 8fc4f00 Compare September 30, 2026 11:55
@tonyredondo
tonyredondo marked this pull request as ready for review September 30, 2026 12:07
@tonyredondo
tonyredondo requested review from a team as code owners September 30, 2026 12:07
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 30, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-23T11:13:47.332848Z 1e14acd Manual request
🔒 Security Review ✅ Completed 2026-10-23T11:11:34.524706Z 1e14acd Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@pr-commenter

pr-commenter Bot commented Sep 30, 2026 •
edited
Loading

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2026-10-23 16:10:31

Comparing candidate commit 94750e5 in PR branch tony/ci-visibility-ipc-cross-runtime-lock with baseline commit 3107870 in branch master.

📊 Benchmarking dashboard

Found 0 performance improvements and 0 performance regressions! Performance is the same for 72 metrics, 0 unstable metrics, 6 known flaky benchmarks, 120 flaky benchmarks without significant changes.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

Known flaky benchmarks

These benchmarks are marked as flaky and will not trigger a failure. Modify FLAKY_BENCHMARKS_REGEX to control which benchmarks are marked as flaky.

scenario:Benchmarks.Trace.AspNetCoreBenchmark.SendRequest net6.0

  • unstable execution_time [+16.853ms; +89.164ms] or [+3.740%; +19.787%]

scenario:Benchmarks.Trace.AspNetCoreBenchmark.SendRequest netcoreapp3.1

  • unstable execution_time [-117.131ms; -37.447ms] or [-25.916%; -8.285%]

scenario:Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces net472

  • unstable execution_time [-60.742ms; +21.120ms] or [-10.509%; +3.654%]
  • unstable throughput [-113.233op/s; -48.524op/s] or [-18.516%; -7.935%]

scenario:Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark net6.0

  • unstable execution_time [-115.302µs; -14.142µs] or [-19.621%; -2.407%]
  • unstable throughput [+0.508op/s; +255.389op/s] or [+0.028%; +14.089%]

scenario:Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark netcoreapp3.1

  • unstable execution_time [-3.125µs; +5.741µs] or [-5.714%; +10.498%]
  • unstable throughput [-1105.027op/s; +782.573op/s] or [-5.934%; +4.203%]

scenario:Benchmarks.Trace.SingleSpanAspNetCoreBenchmark.SingleSpanAspNetCore net6.0

  • unstable execution_time [-74.565ms; -18.548ms] or [-14.894%; -3.705%]

Known flaky benchmarks without significant changes:

  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.ActivityBenchmark.StartSpan net472
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.ActivityBenchmark.StartSpan net6.0
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.ActivityBenchmark.StartSpan netcoreapp3.1
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.ActivityBenchmark.StartSpan_AddEvent_Sampled net472
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.ActivityBenchmark.StartSpan_AddEvent_Sampled net6.0
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.ActivityBenchmark.StartSpan_AddEvent_Sampled netcoreapp3.1
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.ActivityBenchmark.StartSpan_GetContext_Sampled net472
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.ActivityBenchmark.StartSpan_GetContext_Sampled net6.0
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.ActivityBenchmark.StartSpan_GetContext_Sampled netcoreapp3.1
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.ActivityBenchmark.StartSpan_SetAttributes_Sampled net472
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.ActivityBenchmark.StartSpan_SetAttributes_Sampled net6.0
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.ActivityBenchmark.StartSpan_SetAttributes_Sampled netcoreapp3.1
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.ActivityBenchmark.StartSpan_SetStatus_Sampled net472
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.ActivityBenchmark.StartSpan_SetStatus_Sampled net6.0
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.ActivityBenchmark.StartSpan_SetStatus_Sampled netcoreapp3.1
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.ActivityBenchmark.StartSpan_UpdateName_Sampled net472
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.ActivityBenchmark.StartSpan_UpdateName_Sampled net6.0
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.ActivityBenchmark.StartSpan_UpdateName_Sampled netcoreapp3.1
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.TelemetrySpanBenchmark.StartSpan net472
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.TelemetrySpanBenchmark.StartSpan net6.0
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.TelemetrySpanBenchmark.StartSpan netcoreapp3.1
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.TelemetrySpanBenchmark.StartSpan_AddEvent_Sampled net472
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.TelemetrySpanBenchmark.StartSpan_AddEvent_Sampled net6.0
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.TelemetrySpanBenchmark.StartSpan_AddEvent_Sampled netcoreapp3.1
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.TelemetrySpanBenchmark.StartSpan_GetContext_Sampled net472
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.TelemetrySpanBenchmark.StartSpan_GetContext_Sampled net6.0
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.TelemetrySpanBenchmark.StartSpan_GetContext_Sampled netcoreapp3.1
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.TelemetrySpanBenchmark.StartSpan_RecordException_Sampled net472
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.TelemetrySpanBenchmark.StartSpan_RecordException_Sampled net6.0
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.TelemetrySpanBenchmark.StartSpan_RecordException_Sampled netcoreapp3.1
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.TelemetrySpanBenchmark.StartSpan_SetAttributes_Sampled net472
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.TelemetrySpanBenchmark.StartSpan_SetAttributes_Sampled net6.0
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.TelemetrySpanBenchmark.StartSpan_SetAttributes_Sampled netcoreapp3.1
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.TelemetrySpanBenchmark.StartSpan_SetStatus_Sampled net472
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.TelemetrySpanBenchmark.StartSpan_SetStatus_Sampled net6.0
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.TelemetrySpanBenchmark.StartSpan_SetStatus_Sampled netcoreapp3.1
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.TelemetrySpanBenchmark.StartSpan_UpdateName_Sampled net472
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.TelemetrySpanBenchmark.StartSpan_UpdateName_Sampled net6.0
  • scenario:Benchmarks.OpenTelemetry.InstrumentedApi.Trace.TelemetrySpanBenchmark.StartSpan_UpdateName_Sampled netcoreapp3.1
  • scenario:Benchmarks.Trace.ActivityBenchmark.StartStopWithChild net472
  • scenario:Benchmarks.Trace.ActivityBenchmark.StartStopWithChild net6.0
  • scenario:Benchmarks.Trace.ActivityBenchmark.StartStopWithChild netcoreapp3.1
  • scenario:Benchmarks.Trace.AgentWriterBenchmark.WriteAndFlushEnrichedTraces net472
  • scenario:Benchmarks.Trace.AgentWriterBenchmark.WriteAndFlushEnrichedTraces net6.0
  • scenario:Benchmarks.Trace.AgentWriterBenchmark.WriteAndFlushEnrichedTraces netcoreapp3.1
  • scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.AllCycleMoreComplexBody net472
  • scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.AllCycleMoreComplexBody net6.0
  • scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.AllCycleMoreComplexBody netcoreapp3.1
  • scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.AllCycleSimpleBody net472
  • scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.AllCycleSimpleBody net6.0
  • scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.AllCycleSimpleBody netcoreapp3.1
  • scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorMoreComplexBody net472
  • scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorMoreComplexBody net6.0
  • scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorMoreComplexBody netcoreapp3.1
  • scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody net472
  • scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody net6.0
  • scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody netcoreapp3.1
  • scenario:Benchmarks.Trace.Asm.AppSecEncoderBenchmark.EncodeArgs net472
  • scenario:Benchmarks.Trace.Asm.AppSecEncoderBenchmark.EncodeArgs net6.0
  • scenario:Benchmarks.Trace.Asm.AppSecEncoderBenchmark.EncodeArgs netcoreapp3.1
  • scenario:Benchmarks.Trace.Asm.AppSecEncoderBenchmark.EncodeLegacyArgs net472
  • scenario:Benchmarks.Trace.Asm.AppSecEncoderBenchmark.EncodeLegacyArgs net6.0
  • scenario:Benchmarks.Trace.Asm.AppSecEncoderBenchmark.EncodeLegacyArgs netcoreapp3.1
  • scenario:Benchmarks.Trace.Asm.AppSecWafBenchmark.RunWafRealisticBenchmark net472
  • scenario:Benchmarks.Trace.Asm.AppSecWafBenchmark.RunWafRealisticBenchmark net6.0
  • scenario:Benchmarks.Trace.Asm.AppSecWafBenchmark.RunWafRealisticBenchmark netcoreapp3.1
  • scenario:Benchmarks.Trace.Asm.AppSecWafBenchmark.RunWafRealisticBenchmarkWithAttack net472
  • scenario:Benchmarks.Trace.Asm.AppSecWafBenchmark.RunWafRealisticBenchmarkWithAttack net6.0
  • scenario:Benchmarks.Trace.Asm.AppSecWafBenchmark.RunWafRealisticBenchmarkWithAttack netcoreapp3.1
  • scenario:Benchmarks.Trace.AspNetCoreBenchmark.SendRequest net472
  • scenario:Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces net6.0
  • scenario:Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces netcoreapp3.1
  • scenario:Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSlice net472
  • scenario:Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSlice net6.0
  • scenario:Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSlice netcoreapp3.1
  • scenario:Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSliceWithPool net472
  • scenario:Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSliceWithPool net6.0
  • scenario:Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSliceWithPool netcoreapp3.1
  • scenario:Benchmarks.Trace.CharSliceBenchmark.OriginalCharSlice net472
  • scenario:Benchmarks.Trace.CharSliceBenchmark.OriginalCharSlice net6.0
  • scenario:Benchmarks.Trace.CharSliceBenchmark.OriginalCharSlice netcoreapp3.1
  • scenario:Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearch net472
  • scenario:Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearch net6.0
  • scenario:Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearch netcoreapp3.1
  • scenario:Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearchAsync net472
  • scenario:Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearchAsync net6.0
  • scenario:Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearchAsync netcoreapp3.1
  • scenario:Benchmarks.Trace.GraphQLBenchmark.ExecuteAsync net472
  • scenario:Benchmarks.Trace.GraphQLBenchmark.ExecuteAsync net6.0
  • scenario:Benchmarks.Trace.GraphQLBenchmark.ExecuteAsync netcoreapp3.1
  • scenario:Benchmarks.Trace.ILoggerBenchmark.EnrichedLog net472
  • scenario:Benchmarks.Trace.ILoggerBenchmark.EnrichedLog net6.0
  • scenario:Benchmarks.Trace.ILoggerBenchmark.EnrichedLog netcoreapp3.1
  • scenario:Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark net472
  • scenario:Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark netcoreapp3.1
  • scenario:Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark net472
  • scenario:Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark net6.0
  • scenario:Benchmarks.Trace.Log4netBenchmark.EnrichedLog net472
  • scenario:Benchmarks.Trace.Log4netBenchmark.EnrichedLog net6.0
  • scenario:Benchmarks.Trace.Log4netBenchmark.EnrichedLog netcoreapp3.1
  • scenario:Benchmarks.Trace.RedisBenchmark.SendReceive net472
  • scenario:Benchmarks.Trace.RedisBenchmark.SendReceive net6.0
  • scenario:Benchmarks.Trace.RedisBenchmark.SendReceive netcoreapp3.1
  • scenario:Benchmarks.Trace.SerilogBenchmark.EnrichedLog net472
  • scenario:Benchmarks.Trace.SerilogBenchmark.EnrichedLog net6.0
  • scenario:Benchmarks.Trace.SerilogBenchmark.EnrichedLog netcoreapp3.1
  • scenario:Benchmarks.Trace.SingleSpanAspNetCoreBenchmark.SingleSpanAspNetCore net472
  • scenario:Benchmarks.Trace.SingleSpanAspNetCoreBenchmark.SingleSpanAspNetCore netcoreapp3.1
  • scenario:Benchmarks.Trace.SpanBenchmark.StartFinishScope net472
  • scenario:Benchmarks.Trace.SpanBenchmark.StartFinishScope net6.0
  • scenario:Benchmarks.Trace.SpanBenchmark.StartFinishScope netcoreapp3.1
  • scenario:Benchmarks.Trace.SpanBenchmark.StartFinishSpan net472
  • scenario:Benchmarks.Trace.SpanBenchmark.StartFinishSpan net6.0
  • scenario:Benchmarks.Trace.SpanBenchmark.StartFinishSpan netcoreapp3.1
  • scenario:Benchmarks.Trace.SpanBenchmark.StartFinishTwoScopes net472
  • scenario:Benchmarks.Trace.SpanBenchmark.StartFinishTwoScopes net6.0
  • scenario:Benchmarks.Trace.SpanBenchmark.StartFinishTwoScopes netcoreapp3.1
  • scenario:Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin net472
  • scenario:Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin net6.0
  • scenario:Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin netcoreapp3.1

@dd-trace-dotnet-ci-bot

dd-trace-dotnet-ci-bot Bot commented Sep 30, 2026 •
edited
Loading

Copy link
Copy Markdown

Execution-Time Benchmarks Report ⏱️

Execution-time results for samples comparing This PR (9335) and master.

✅ No regressions detected

📄 View the full report (charts + all metrics) →

@tonyredondo
tonyredondo force-pushed the tony/ci-visibility-ipc-cross-runtime-lock branch from 8fc4f00 to ebb8189 Compare September 30, 2026 16:28
@tonyredondo
tonyredondo force-pushed the tony/ci-visibility-ipc-cross-runtime-lock branch from ebb8189 to 1e14acd Compare October 23, 2026 10:42
@tonyredondo tonyredondo mentioned this pull request Oct 23, 2026
Open
@andrewlock

andrewlock commented Oct 23, 2026

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 23, 2026

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: 1e14acdcb5

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Oct 23, 2026
View reviewed changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1e14acdcb5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tracer/test/Datadog.Trace.Tests/Ci/Ipc/CircularChannelTests.cs Outdated
@tonyredondo
tonyredondo force-pushed the tony/ci-visibility-ipc-cross-runtime-lock branch from 1e14acd to bf0693b Compare October 23, 2026 12:05
@tonyredondo tonyredondo changed the title [CI Visibility] Use a cross-runtime file lock for the IPC channel on Unix [CI Visibility] Fix message loss and abandoned mutexes in the IPC channel Oct 23, 2026
@tonyredondo
[CI Visibility] Keep unread IPC messages when a channel is opened
407cd70 
Opening a CircularChannel reset its read and write pointers, so every new
IPC client dropped the messages the other side hadn't read yet. Only a new
file, or one with an unexpected size, is initialized now.
@tonyredondo
[CI Visibility] Never fill the IPC channel completely
a0ce0ba 
A completely full buffer was marked with a virtual write position
(BufferBodySize + writePos), which overflows the ushort pointer with the
default 64 KB buffer. The reader then looped over the same messages forever
while holding the lock, until the process ran out of memory. Always keep
one byte free instead, so the write position never reaches the read position.
@tonyredondo
tonyredondo force-pushed the tony/ci-visibility-ipc-cross-runtime-lock branch from bf0693b to a84c18a Compare October 23, 2026 13:38
@tonyredondo
[CI Visibility] Acquire abandoned IPC mutexes when opening a channel …
f28095a 
…or writing

WaitOne grants ownership of an abandoned mutex before it throws
AbandonedMutexException. The CircularChannel reader already handled that,
but the constructor and the writer let the exception escape, so the call
failed and the thread kept owning the mutex, which blocked every other
process using the channel. All three paths now share the reader's handling.

This resolves the TODO in CheckBuildLogsForErrors, so the ignore patterns
for those errors are removed. The pattern for ReleaseMutex failing after a
successful wait stays: that one comes from the .NET 11 RC named mutex bug
(dotnet/runtime#134491) and goes away with .NET 11 GA.
@tonyredondo
[CI Visibility] Read the messages written before a reader is disposed
94750e5 
The reader slept between polls and returned early once the reader or the
channel was disposed, so anything written during its last polling
interval was dropped. A session that disposes its IPC server right after
the last message from a child process could lose that message.

The reader now waits on the stop event instead of sleeping, so Dispose
wakes it right away, and it polls once more before it stops. Dispose
already joins the polling thread before the channel releases the memory
mapped file.
@tonyredondo
tonyredondo force-pushed the tony/ci-visibility-ipc-cross-runtime-lock branch from a84c18a to 94750e5 Compare October 23, 2026 15:16

This branch has not been deployed

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

Reviewers

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments
@gnufede gnufede gnufede approved these changes
@anmarchenko anmarchenko anmarchenko approved these changes

Assignees

No one assigned

Labels

area:ci-visibility

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@tonyredondo @andrewlock @gnufede @anmarchenko