Skip to content

[EventPipe] Add Non-Lossy EventPipe Mode for gcdump#129457

Open
mdh1418 wants to merge 22 commits into
dotnet:mainfrom
mdh1418:eventpipe-nonlossy-block
Open

[EventPipe] Add Non-Lossy EventPipe Mode for gcdump#129457
mdh1418 wants to merge 22 commits into
dotnet:mainfrom
mdh1418:eventpipe-nonlossy-block

Conversation

@mdh1418

@mdh1418 mdh1418 commented Jun 16, 2026

Copy link
Copy Markdown
Member

Runtime side to fix dotnet/diagnostics#2404.

Problem

EventPipe's buffer manager is lossy by design: when a producer's per-thread buffer is full
and the circular pool is exhausted, the event is dropped (accounted via a sequence number)
rather than blocking the writer.

This corrupts dotnet-gcdump. A heap snapshot is rebuilt from the GCBulkNode/GCBulkEdge
stream emitted during a stop-the-world heap walk; on a large heap that burst overruns the buffer,
edges are dropped, and the graph cannot be reconstructed - gcdump fails with
System.ApplicationException: RootIndex not set and writes no dump (dotnet/diagnostics#2404).

This PR adds an opt-in, per-session EventPipeBufferingMode that defaults to DROP (today's
behavior) and can be set to BLOCK to make a streaming session non-lossy. Two opt-ins are
included - the IPC CollectTracing command (used by dotnet-gcdump --non-lossy) and a
DOTNET_EventPipeBufferingMode environment variable for the startup session.

Parking producers instead of dropping

The non-lossy guarantee comes down to one change in the write path: when a producer in Block mode
has filled its buffer and the pool cannot hand it another, instead of dropping the event it
parks until the reader frees capacity and then retries the same write. Parked producers wait
in a strict-FIFO queue; only the thread at the front may reserve the next freed buffer, each
thread waits on its own event, and the reader wakes the front waiter every time it returns a
drained buffer to the pool. A parked producer makes forward progress only once the event actually
lands.

Parking has to cooperate with the existing buffer hand-off, which is where a new pin flag comes
in. Each producer advertises, in its per-thread session_use_in_progress slot, the index of the
session it is writing to; teardown spins on that slot to keep the session and its buffer manager
alive until every in-flight writer is done. We widen that slot with a high
EP_SESSION_USE_WRITE_BUFFER_IN_USE bit meaning "I currently own a write buffer," and the reader
drains a producer's buffer only once that bit is clear. A parked producer therefore clears the
bit but keeps the index
: clearing the bit lets the reader drain the very buffer the producer
just filled - the only way capacity is ever freed - while keeping the index leaves the session
pinned so it cannot be torn down from under the sleeping thread. On wake the producer re-sets the
bit, reloads the session pointer (disable may have cleared it), and retries.

Blocking is only ever allowed when it is actually safe to wait for a reader: the session is in
Block mode, it is not tearing down, and the caller is not the rundown thread. Teardown is
the first escape hatch - disable raises an aborting flag and wakes every parked producer, and
the producer, which re-checks aborting immediately before and after each wait, gives up and
drops the event, accounted via its sequence number like any other drop, rather than waiting on a reader that is going away. Rundown is the second - rundown events
are emitted from the drain side during disable, so a rundown writer that blocked would be waiting
on itself; it is excluded from blocking and falls back to the normal drop path.

Finally, Block waits only for buffer capacity, which a reader is guaranteed to free. A genuine allocation failure (e.g. OOM allocating a buffer's backing memory) is not a capacity wait and has no reader-driven wakeup, so it is a recorded drop (accounted via the sequence number), not a park. Block is therefore non-lossy up to buffer capacity, not against host memory exhaustion; a diagnostics feature must never be able to hang the app it observes.

Expressing these three outcomes cleanly is why the write path's old success/fail boolean became an
EventPipeWriteEventResult enum - WRITTEN (the event landed), DROPPED (discarded:
oversized, provider disabled, suspended, an allocation failure (e.g. OOM), or a genuine Drop-mode overflow, all already tracked by
sequence numbers), and BLOCKED (Block mode, buffer full, session still live - park and retry).
Only BLOCKED drives the park loop.

Starting the drain thread before the heap walk

Enabling a session invokes each provider's enable callback. For a gcdump (GCHeapSnapshot)
session that callback synchronously forces a full, stop-the-world heap walk that emits the entire
GCBulkNode/GCBulkEdge stream - which in Block mode is exactly what fills the buffer and parks
the producer, and the producer can only make progress if a drain thread already exists. Originally
the callbacks fired inline inside ep_enable_3, before streaming starts, so the walk ran during
SuspendEE with no drainer: the buffer filled, the heap-walk thread parked forever, and the
target deadlocked.

Two changes fix the ordering:

  • session_init / enable split. ep_enable_3 now only publishes the session inert
    (session_init) and callback dispatch moves to a single site in ep_start_session, which
    collects the provider-enable callbacks under the lock, starts the drain thread, waits for it
    to be running
    (EP_YIELD_WHILE (!ep_session_has_started)), and only then dispatches them.
    A consumer is therefore guaranteed to be in place before the walk can fill the buffer, and the
    drain loop runs in preemptive GC mode, so it keeps draining even while the heap walk holds
    the EE suspended.

  • Native, eager drain threads. The session drain thread is now a raw native thread (like the
    diagnostics server thread) instead of a managed CLR thread, so it needs no GC / Thread Store and
    starts eagerly at enable time. This removes the previous startup deferral (sessions enabled
    early no longer wait until ep_finish_init to start streaming), so even a startup gcdump-style
    session has a live drainer before its heap walk runs.

Where Block is allowed

Block parks a producer until a drain frees capacity, so it is only safe for session types that
have a continuous native drain thread: FILESTREAM and IPCSTREAM. FILE and LISTENER
sessions use the buffer manager but have no streaming thread (a FILE session flushes only at
disable; a LISTENER session is pumped by an in-proc managed poll), and SYNCHRONOUS /
USEREVENTS have no buffer manager. A central guard in ep_session_alloc therefore rejects
Block
for any session type without a streaming thread (the request is an unsupported configuration) and the session fails to start, so a request can never deadlock a
session that has no drainer - regardless of how the mode was requested.

The two opt-ins:

  • IPC - CollectTracing6 (0x0207). Extends CollectTracing5 with a trailing uint32
    buffering mode for streaming (IPCSTREAM) sessions: 0 = Drop, 1 = Block; any other value is rejected so an unsupported or future mode can't silently become Drop. Older command
    versions are unchanged. dotnet-gcdump --non-lossy sends it (dotnet/diagnostics).

  • Env var - DOTNET_EventPipeBufferingMode. Lets the DOTNET_EnableEventPipe startup session
    opt in: 0 = Drop (default), 1 = Block; any other value is an invalid configuration and the startup session does not start, rather than silently degrading to a different mode. Block only applies to a streaming session (DOTNET_EventPipeOutputStreaming=1, i.e. FILESTREAM); requesting it on a non-streaming FILE session is the unsupported configuration the guard above rejects, so that startup session fails to start. Wired on CoreCLR and NativeAOT (Mono stubs the getter to Drop).

Managed (EventListener), profiler, and gen-analysis sessions go through paths that always pass
Drop and are unaffected.

Single-threaded builds

Under PERFTRACING_DISABLE_THREADS (single-threaded, e.g. browser WASM) there is no separate
drain thread - the drain runs cooperatively on the same thread - and ep_rt_wait_event_wait is a
no-op, so a parked producer could never be woken. The Block park/abort/fair-reserve machinery is
#ifndef-d out of those builds, and a BLOCK request degrades to the existing Drop path.

Testing

Validated on a Checked CoreCLR build (asserts on; none fired). CoreCLR Checked builds clean
(0 warnings, 0 errors).

End-to-end gcdump (the actual use case) - dotnet-gcdump --non-lossy (which sends
CollectTracing6 with Block) against a target holding a 2,000,000-node object graph: the command
is accepted, the heap walk completes, and the report reconstructs exactly 2,000,000 nodes with
exit 0 and no hang/deadlock.

Block vs Drop differential - a startup FILESTREAM session with a 1 MB circular buffer
(DOTNET_EnableEventPipe=1, DOTNET_EventPipeOutputStreaming=1,
DOTNET_EventPipeBufferingMode), target emits 1,000,000 events as fast as possible:

Drop (=0) Block (=1)
Tick events captured 384,976 / 1,000,000 (61.5% dropped) 1,000,000 / 1,000,000 (lossless)

Guard - a non-streaming FILE session (DOTNET_EventPipeOutputStreaming=0) with
DOTNET_EventPipeBufferingMode=1 is rejected as an unsupported configuration: the startup session
fails to start (no trace), and the process runs normally with no hang, rather than parking a producer that has no drainer.

Introduce EventPipeBufferingMode (Drop/Block) and carry it as a per-session
opt-in on EventPipeSessionOptions (default Drop), plumbed through enable() and
ep_session_alloc onto the buffer manager. Block is wired up here but not yet
acted on; producers start parking on full buffers in a later change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR extends native EventPipe to support an opt-in non-lossy (“blocking”) buffering mode intended for GC heap snapshot (gcdump) scenarios, and reworks provider-enable callback dispatch to avoid blocking producers before a drain thread exists.

Changes:

  • Introduces EventPipeBufferingMode (DROP/BLOCK) and EventPipeWriteEventResult (WRITTEN/DROPPED/BLOCKED), plumbing the buffering mode through session options to the buffer manager.
  • Adds a block-and-retry path in the write pipeline: writers may park on buffer exhaustion in BLOCK mode, and teardown can abort/wake parked writers.
  • Defers provider enable-callback invocation to ep_start_streaming, storing callbacks on the session until streaming begins.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
src/native/eventpipe/ep.h Adds buffering_mode to EventPipeSessionOptions.
src/native/eventpipe/ep.c Implements blocking retry loop, abort-on-disable, and deferred provider callback dispatch.
src/native/eventpipe/ep-types-forward.h Adds new enums for buffering mode and write results.
src/native/eventpipe/ep-thread.h Adds EP_SESSION_USE_WRITE_BUFFER_IN_USE bit for writer/reader coordination.
src/native/eventpipe/ep-thread.c Adjusts assertions to account for the new session-use bit.
src/native/eventpipe/ep-session.h Stores provider callback queue on session; changes ep_session_write_event return type.
src/native/eventpipe/ep-session.c Plumbs buffering mode into buffer manager; updates inflight-wait logic; updates write path to return EventPipeWriteEventResult.
src/native/eventpipe/ep-buffer-manager.h Adds BLOCK-mode fields/APIs; changes write API to return EventPipeWriteEventResult.
src/native/eventpipe/ep-buffer-manager.c Implements BLOCK-mode signaling/aborting and returns BLOCKED on buffer exhaustion when appropriate.
src/mono/mono/eventpipe/test/ep-tests.c Updates tests for new ep_session_write_event return type (currently has incorrect enum constant usages).
src/mono/mono/eventpipe/test/ep-buffer-manager-tests.c Updates tests for new ep_buffer_manager_write_event return type (currently has incorrect enum constant usage).

Comment thread src/mono/mono/eventpipe/test/ep-tests.c Outdated
Comment thread src/mono/mono/eventpipe/test/ep-tests.c Outdated
Comment thread src/mono/mono/eventpipe/test/ep-tests.c Outdated
Comment thread src/mono/mono/eventpipe/test/ep-tests.c Outdated
Comment thread src/mono/mono/eventpipe/test/ep-tests.c Outdated
Comment thread src/mono/mono/eventpipe/test/ep-buffer-manager-tests.c Outdated
Comment thread src/native/eventpipe/ep.c Outdated
mdh1418 and others added 3 commits June 16, 2026 15:04
In Block mode a producer that finds the buffers full parks until the reader
frees capacity and then retries, instead of dropping the event. The reader
signals a per-buffer-manager auto-reset event on each capacity release; a
parked producer clears the WRITE_BUFFER_IN_USE bit of its
session_use_in_progress (keeping the session index) so the reader can drain
its full buffer while the retained index pins the buffer manager. Teardown
raises an abort flag and reuses the existing in-flight-write wait to release
parked producers, so no separate parked-writer count is needed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
enable()/ep_enable_3 only sets the session up and collects its provider-enable
callbacks onto the session; ep_start_streaming is now the single site that
invokes them, for every session.

This is required for a blocking GCHeapSnapshot session: its provider-enable
callback triggers a GC heap walk that parks producers when the buffer fills, so
it must run only after the drain thread is live to consume - otherwise the walk
parks with no reader and deadlocks. ep_start_streaming waits for the drain
thread to reach its preemptive drain loop before invoking the callbacks; when
streaming is parked for ep_finish_init at early startup it invokes inline, which
is safe because the heap walk no-ops until the GC is fully initialized.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ep_start_streaming invoked the provider-enable callbacks that enable()
collected onto the session *after* leaving the EventPipe lock, reading,
clearing, and freeing session->provider_callbacks with no synchronization.
A concurrent disable - most reachably the session's own streaming thread
self-disabling on a write failure - runs ep_session_dec_ref under the lock
and frees that same queue, so the two paths could race into a
use-after-free / double-free.

Detach the queue under the lock instead (grab the pointer and NULL the
field), then invoke and free that private copy outside the lock. Ownership
is now unambiguous: a concurrent disable's ep_session_dec_ref sees NULL and
leaves the queue alone, and its defensive drain/free still covers the case
where a session is disabled before ep_start_streaming runs. The single-use
dispatch helper is inlined now that it just drains a local queue.

The has_started spin still reads the session outside the lock (it must -
waiting under the lock could deadlock the drain thread startup against a GC
holding the thread-store lock), but that read is safe: disable joins the
drain thread before freeing the session, and the thread publishes "started"
before it can exit.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mdh1418 mdh1418 force-pushed the eventpipe-nonlossy-block branch from c7a287d to f15cb28 Compare June 16, 2026 19:05
Comment thread src/native/eventpipe/ep-buffer-manager.c Outdated
Comment thread src/native/eventpipe/ep-buffer-manager.c Outdated
Comment thread src/native/eventpipe/ep-buffer-manager.c
Comment thread src/native/eventpipe/ep-buffer-manager.c Outdated
Comment thread src/native/eventpipe/ep-session.c Outdated
Comment thread src/native/eventpipe/ep-session.c Outdated
Comment thread src/native/eventpipe/ep.c Outdated
Comment thread src/native/eventpipe/ep.c Outdated
Comment thread src/mono/mono/eventpipe/test/ep-buffer-manager-tests.c
mdh1418 and others added 2 commits June 17, 2026 21:46
- Reclaim a buffer's memory before releasing its budget (now a single helper), so the Block-mode capacity signal is only observed once the memory is freed - matching the buffer-allocation error path.

- ep_buffer_manager_abort_blocked_writers only raises the abort flag. Waking parked producers is owned by ep_session_wait_for_inflight_thread_ops, which signals capacity each iteration until every producer observes the abort, so the previous lone signal was redundant and racy and is removed.

- ep_session_wait_for_inflight_thread_ops selects the Block-mode wait from the buffer manager's immutable buffering mode and asserts the abort flag is set, instead of branching on a racy field.

- Fold write_event_2's first send and its Block-mode park/retry into one loop with a single ep_session_write_event call.

- When a session is disabled before ep_start_streaming dispatched its provider-enable callbacks, move them into the disable callback queue so they still fire, balanced with the disable notifications, and so each provider's callbacks_pending is decremented - otherwise ep_delete_provider can wait on it forever.

- Assert the Block-only buffer_available_event is valid where it is waited on and signaled.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds test_buffer_manager_block_mode_abort_and_disable to the native EventPipe buffer-manager unit tests, covering the non-lossy Block buffering path:

- fill a 1 MB buffer pool until ep_buffer_manager_write_event returns EP_WRITE_EVENT_RESULT_BLOCKED, verifying a full buffer parks the producer (park-and-retry) instead of dropping the event;

- abort the blocked writers and verify a subsequent write gives up (no longer BLOCKED) rather than parking;

- verify the capacity signal/wait wake path returns without hanging;

- tear the Block-mode session down via ep_session_dec_ref.

Supporting changes in the same test file:

- Refactor buffer_manager_init into buffer_manager_init_mode (parameterized by buffering mode); buffer_manager_init forwards EP_BUFFERING_MODE_DROP so existing callers are unchanged. This also corrects a pre-existing arg misalignment in the shared ep_session_alloc call (it was missing circular_buffer_size_in_mb and user_events_data_fd).

- write_events now publishes the session index via ep_thread_set_session_use_in_progress before writing, satisfying the producer-index assert that Block mode added to ep_buffer_manager_write_event.

NOTE: this native EventPipe unit-test target is not compiled in CI, so the test is not compile- or run-verified here.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@noahfalk

Copy link
Copy Markdown
Member

So I think this new feature should not compile under PERFTRACING_DISABLE_THREADS.

Alternatively it could write-thru (there is no other thread that would conflict). The underlying web-socket or JS buffer never blocks. But that would probably need more testing.

Either of these sound like good solutions to me. 👍

ep_start_streaming does the full "start the session" work - configure
providers, set allow_write, bump the session count, init the sample profiler,
start the drain thread, and dispatch the provider-enable callbacks - so
"streaming" undersold it. ep_start_session names the global enable driver after
what it does and pairs cleanly with the existing stop_session disable driver
(ep_enable* / ep_start_session to bring a session up, ep_disable -> stop_session
to take it down).

ep_start_session keeps the ep_ prefix (a non-static, cross-module EventPipe API
declared in ep.h) and deliberately avoids the ep_session_ prefix, which is
reserved for operations on a single EventPipeSession object; the session-object
drain-thread starter ep_session_start_streaming is unchanged.

Mechanical rename across the call sites: ep.h, the IPC collect handler, the
env-var startup path, EventPipeAdapter::StartStreaming (CoreCLR), the NativeAOT
QCall, the Mono component vtable entry, and the Mono EventPipe tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 25, 2026 19:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 3 comments.

Comment thread src/native/eventpipe/ds-eventpipe-protocol.c Outdated
Comment thread src/native/eventpipe/ep-buffer-manager.c Outdated
Comment thread src/mono/mono/eventpipe/test/ep-buffer-manager-tests.c Outdated
Copilot AI review requested due to automatic review settings June 25, 2026 19:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 4 comments.

Comment thread src/native/eventpipe/ds-eventpipe-protocol.c Outdated
Comment thread src/native/eventpipe/ep.c Outdated
Comment thread src/mono/mono/eventpipe/test/ep-buffer-manager-tests.c
Comment thread src/native/eventpipe/ep-session.c Outdated
mdh1418 and others added 2 commits June 25, 2026 18:42
- Allocate the Block-mode buffer manager test session as FILESTREAM so the streaming-only guard keeps Block enabled (FILE would be coerced to Drop).

- Map only EventPipeBufferingMode value 1 (EP_BUFFERING_MODE_BLOCK) to Block in the CollectTracing6 parser and the env-var path; any other value falls back to Drop. Update the config description to match.

- Read buffer_wait_enqueued under the buffer manager rt_lock in ep_buffer_manager_writer_wait_for_capacity, matching the field's documented locking; the wait itself stays outside the lock.

- Free the provider-callback queue from ep_on_exit so it also runs on the error path (ep_provider_callback_data_queue_fini already null-checks its argument).

- Clarify the paused-session write-result comment.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…d NativeAOT

The drain thread now starts eagerly, so it can be created and torn down
while the runtime is suspended at startup (diagnostic-port pause), before
the GC/finalizer and ThreadStore exist. A full managed attach faults there:
NativeAOT's AttachCurrentThread dereferences the null ThreadStore when a
session starts, and Mono's managed detach hits the uninitialized finalizer
semaphore when a session stops.

Attach the SESSION drain thread at the minimum its native drain loop needs:
none on NativeAOT (like CoreCLR), thread-info only on Mono (a MonoThreadInfo
for the coop-GC primitives, without a managed MonoInternalThread whose detach
notifies the finalizer). The sampling thread keeps its full managed attach
since it walks managed stacks.

Fixes the pauseonstart start/stop-while-paused regressions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 6, 2026 22:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread src/native/eventpipe/ep.c
Comment thread src/native/eventpipe/ep-buffer-manager.h

@noahfalk noahfalk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall looks pretty good - comments inline on a variety of smaller cleanup.

Comment thread src/native/eventpipe/ds-eventpipe-protocol.c Outdated
Comment thread src/native/eventpipe/ep-buffer-manager.c Outdated
Comment thread src/native/eventpipe/ep-buffer-manager.c Outdated
Comment thread src/native/eventpipe/ep-buffer-manager.c Outdated
Comment thread src/native/eventpipe/ep-session.c Outdated
Comment thread src/native/eventpipe/ep.c Outdated
Comment thread src/native/eventpipe/ep.c Outdated
Comment thread src/native/eventpipe/ep.c Outdated
Comment thread src/native/eventpipe/ep.c Outdated
Comment thread src/native/eventpipe/ep.c Outdated
mdh1418 and others added 2 commits July 8, 2026 15:57
Remove the two Block-mode allocation-failure fallback paths:
- Replace the dn_queue wait queue with an intrusive singly-linked list
  threaded through EventPipeThread::buffer_wait_next_thread (head/tail on
  the buffer manager), so enqueuing never allocates.
- Allocate the per-thread park event when the thread first gets a Block-mode
  session state, folding any failure into the existing drop path, so the
  write/park path can assume it exists.

Other feedback:
- Drop the eager wait in ep_start_session; the drain thread frees capacity
  and wakes parked producers via deallocate_buffer regardless of whether it
  has finished starting, so no pre-wait is needed.
- Reject Block on non-streaming session types in ep_session_alloc instead of
  silently degrading to Drop.
- Remove the write-only session->streaming_thread field and the vestigial
  has_started check in the drain-thread func.
- Move the was_enabled drain out of session_fini into disable_holding_lock so
  session_fini is a pure mirror of session init.
- Guard the callback-queue OOM in ep_start_session; share one CollectTracing
  v5/v6 parser; collapse the duplicated block-mode write loop; comment and
  naming cleanup.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
These driver-level entry points now only allocate and publish an inert
session (session_init); the caller separately calls ep_start_session to
enable and start it, so "enable" was misleading. Rename to ep_init_session*
to reflect that they init. The ep_session_* namespace is reserved for methods
operating on an EventPipeSession object, so these driver-level entry points
use ep_init_session* rather than ep_session_init*.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 8, 2026 20:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread src/native/eventpipe/ep-session.c
Comment thread src/native/eventpipe/ep.c
…nt teardown drops

Only Drop (0) and Block (1) are accepted as buffering modes; any other value is an
invalid configuration and no session is started, on both opt-in paths.

- ds-eventpipe-protocol.c: reject unknown CollectTracing6 buffering-mode values
  (only 0=Drop and 1=Block accepted), mirroring the serialization-format range
  check, so an unsupported/future mode fails the command (BAD_ENCODING) instead
  of silently becoming Drop.
- ep.c: apply the same rule to the DOTNET_EventPipeBufferingMode startup session -
  an invalid value starts no session rather than silently coercing to Drop. Also
  bump the thread sequence number when a Block-mode writer gives up a parked event
  during teardown, so the drop stays visible to sequence-number based drop
  detection like the buffer manager's terminal drop paths.
- clrconfigvalues.h: document that only 0/1 are valid (any other value won't start
  the session) and that Block on a non-streaming file session is likewise
  unsupported and fails to start the session.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 8, 2026 22:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread src/native/eventpipe/ep-buffer-manager.c
Comment thread src/native/eventpipe/ep-buffer-manager.c Outdated
buffer_manager_allocate_buffer_for_thread returned NULL for any failure and the
caller treated every NULL as EP_WRITE_EVENT_RESULT_BLOCKED. Parking is only valid
when the writer was enqueued on the fair wait queue (capacity exhausted) - a reader
will wake it. Other failures (e.g. OOM allocating the buffer bytes) are not enqueued
and have no wakeup, so parking them hung the producer until teardown.

Give the allocate out-parameter real meaning (should_block, set only on the
fair-reserve failure path) and gate BLOCKED on it; every other allocation failure
falls through to the recorded drop path. Block is non-lossy up to buffer capacity,
not against host memory exhaustion.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 9, 2026 00:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread src/native/eventpipe/ep.c
Comment thread src/native/eventpipe/ep-session.c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

gcdump fails to collect dump under high load.

5 participants