AdapterBase.Stop() can drop a trace event that's still in flight when called
- Dominant language
- TypeScript
- Stars
- 188
- Forks
- 8
- PR merge metrics
- No merged PRs in 30d
Description
`AdapterBase`'s consumer goroutine (in `Start()`) races the trace-event channel against the stop signal:
```go
go func() {
for {
select {
case <-ctx.Done():
return
case event := <-b.TraceEvents:
a.HandleTraceEvent(event)
case <-b.stop:
return
}
}
}()
```
`Stop()` sends to `b.stop` immediately:
```go
func (b *AdapterBase) Stop(wait bool) {
b.stop <- true
if wait {
b.eventBucket.Wait()
}
}
```
If a trace event is still being finalized and pushed onto the buffered `TraceEvents` channel when `Stop()` is called, there's a real race between that send and `stop` arriving at the `select`. When `stop` wins, the goroutine returns immediately - the pending event is never read, never reaches `HandleTraceEvent`/the event bucket, and is silently lost. `Stop(true)`'s `wait` guarantee (waiting for the event bucket to flush) doesn't help here, since the event never made it into the bucket in the first place.
**Reproduction:** call a wasm function through an instrumented plugin, then call `adapter.Stop(true)` right after the call returns (instead of sleeping an arbitrary amount of time first). The trace for that call is dropped intermittently/reliably depending on timing - because "the call returned" and "the trace event has been pushed to `TraceEvents`" aren't actually synchronized with each other.
This is separate from the `EventBucket.Wait()`/`Add()` ordering issue (a related but distinct bug in `bucket.go`, which I'm sending as its own fix).
**Possible directions** (not proposing a specific one - this touches real design tradeoffs for the project): `Stop()` could drain `TraceEvents` until empty before honoring `stop`, or `HandleTraceEvent`/the trace-finalization path could give the caller an explicit "this instance's events are all queued" signal to wait on before calling `Stop()` at all.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by inspecting AdapterBase.Start(), AdapterBase.Stop(), the TraceEvents channel, and the consumer goroutine shown in the issue; reproduce the race by calling an instrumented wasm function and immediately invoking Stop(true). Compare the shutdown behavior with the event-finalization path, and consider the stated drain or explicit-queueing alternatives. Done means Stop(true) no longer loses an event that was still being queued.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, wasm
- Domain
- observability
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100