Skip to content

add state to socketLink to prevent errors - #286

Merged
brycekahle merged 2 commits into
mainfrom
bryce.kahle/fix-socket-detach
Sep 3, 2026
Merged

add state to socketLink to prevent errors#286
brycekahle merged 2 commits into
mainfrom
bryce.kahle/fix-socket-detach

Conversation

@brycekahle

Copy link
Copy Markdown
Member

What does this PR do?

Adds attached state to socketLink

Motivation

If you called Pause() then Close(), it would result in an error

Additional Notes

Describe how to test your changes

If you called Pause() then Close(), it would result in an error
@brycekahle
brycekahle requested a review from a team as a code owner September 1, 2026 19:27
Comment thread socket.go Outdated
Comment thread socket.go
if errors.Is(err, unix.ENOENT) {
s.attached = false
}
return err

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.

suggestion (non-blocking): the ENOENT branch reconciles state to attached=false but still returns the error, so an already-detached socket surfaces a spurious error through Close()/Pause() into the joined result at probe.go:703. Not a regression, and only reachable when kernel and tracked state diverge, but returning the error here works against the idempotent-detach intent of this PR. If that divergence is the expected "already detached" case, return nil after clearing the flag:

if errors.Is(err, unix.ENOENT) {
	s.attached = false
	return nil
}
return err

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think surfacing the error when kernel and tracked state diverge is important

@brycekahle
brycekahle requested a review from mbertrone September 2, 2026 16:25
@brycekahle
brycekahle merged commit e33e29a into main Sep 3, 2026
3 checks passed
@brycekahle
brycekahle deleted the bryce.kahle/fix-socket-detach branch September 3, 2026 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants