cloudwego / cloudwego/eino

[Bug] compose: receiveWithDeadline writes its named results from a goroutine, racing the deadline return

Open Beginner friendly
#1,279 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
13k
Forks
1.1k
Avg merge
4h 6m
Merged PRs (30d)
41

Description

**Describe the bug**

`compose.receiveWithDeadline` spawns a goroutine that assigns straight into the enclosing function's *named return values*:

```go
go func() {
ta, closed = recv() // writes the caller-visible results
resultCh <- struct{}{}
}()
```

When the deadline fires before `recv()` returns, the function returns through the timeout branch (`return nil, false, true`) while the abandoned goroutine is still running. If the pending receive then completes, the goroutine writes `ta`/`closed` concurrently with that return, and the caller's read of the results is unordered with it as well. Nothing synchronizes the two: in the timeout branch the goroutine's write and the function's return have no happens-before edge, so this is a data race on every deadline expiry where the pending task finishes after the deadline.

The sibling function in the same file does not have this problem — `receiveWithListening` sends a `pair{ta, closed}` through a buffered channel instead of assigning the named results. `receiveWithDeadline` looks like an oversight of the same pattern.

**To Reproduce**

```go
release := make(chan struct{})
recv := func() (*task, bool) {
<-release
return &task{nodeKey: "n"}, true
}

// deadline fires first: returns nil, false, true
_, _, canceled := receiveWithDeadline(recv, time.Now().Add(20*time.Millisecond))
_ = canceled

// the abandoned goroutine now writes ta/closed, after the function returned
close(release)
```

A test doing exactly this is what `go test -race ./compose/` reports as a data race on the named results against the previous implementation. The PR that follows adds that test.

**Expected behavior**

The deadline path should not share memory with the abandoned goroutine; the result should travel over the channel (as `receiveWithListening` already does), so the timeout return is race-free.

**Version:**

v0.9.x (`compose/graph_manager.go` at `main`).

**Environment:**

```
$ go version
go version go1.26.2 windows/amd64
```

**Additional context**

- `receiveWithDeadline` currently has **no test at all**; `receiveWithListening` has four (`TestReceiveWithListening_*` in `compose/graph_manager_test.go`), which is why this went unnoticed.
- Reachable whenever `taskManager.deadline != nil`, i.e. the "already canceled, receive within a grace period" path — the same area as #1149 and #1225.
- Secondary: the function also uses `time.After(timeout)`, which keeps the timer in the runtime heap when `recv` wins; `time.NewTimer` + `Stop()` releases it.

Contributor guide

Open the contributing guide

Research direction

Start with `receiveWithDeadline` in `compose/graph_manager.go` and compare it with `receiveWithListening`, which the issue identifies as the safe sibling pattern. Add a test in `compose/graph_manager_test.go` that lets the deadline return before the pending receive completes, then run `go test -race ./compose/`; done means the test passes without a data race.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
82/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.