envd: tag-based process lookup aborts early due to inverted Range return values
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 1.6k
- Forks
- 438
- PR merge metrics
- No merged PRs in 30d
Description
Summary
In packages/envd/internal/services/process/service.go, getProcess looks up a process by tag by iterating the process map with Map.Range. The boolean return values in the Range callback are inverted relative to sync.Map.Range semantics (return true to continue, false to stop), so the search can terminate before it ever reaches the matching process.
Affected code
case *rpc.ProcessSelector_Tag:
tag := selector.GetTag()
s.processes.Range(func(_ uint32, value *handler.Handler) bool {
if value.Tag == nil {
return true
}
if *value.Tag == tag {
proc = value
return true // match found, but KEEPS iterating
}
return false // non-matching tagged process -> STOPS the whole scan
})
if proc == nil {
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("process with tag %s not found", tag))
}
Map.Range is a thin wrapper around sync.Map.Range, whose contract is: the callback returns true to continue iteration and false to stop.
Root cause
The return values are reversed:
- On a match, the callback returns
true, so iteration continues unnecessarily (and, if multiple processes shared a tag,procwould be overwritten by a later one). - On a tagged but non-matching process, the callback returns
false, which aborts the entire iteration.
Because sync.Map.Range visits entries in an unspecified (effectively random) order, if any non-matching tagged process is visited before the target, the scan stops early, proc stays nil, and the call wrongly returns CodeNotFound ("process with tag X not found") even though a process with that tag exists.
Impact
Tag-based process lookup (Connect / SendInput / SendSignal / Update / etc. via ProcessSelector.tag) becomes order-dependent and can intermittently fail with NotFound whenever more than one tagged process exists concurrently. With a single tagged process the bug is masked, which likely explains why it has gone unnoticed.
Proposed fix
Invert the return values so iteration stops on a match and continues otherwise:
s.processes.Range(func(_ uint32, value *handler.Handler) bool {
if value.Tag != nil && *value.Tag == tag {
proc = value
return false // found it, stop iterating
}
return true // keep looking
})
Notes
This logic has existed since the process cgroup change (#1580) and was never modified afterward (a later lint-only PR just added a blank line). Per the envd contributing guidelines, the fix should also bump packages/envd/pkg/version.go since it changes behavior.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in packages/envd/internal/services/process/service.go at getProcess and review the tag selector callback against sync.Map.Range semantics. Check packages/envd/pkg/version.go for the required behavior-version bump. Confirm that tag lookup continues past non-matching processes and stops at a match, then run the relevant envd tests if available.
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
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100