fix: exit when FUSE errors on accept #2257
Conversation
hessjcg
force-pushed
the
gh-2256-fuse-exit
branch
3 times, most recently
from
73a065c
to
c44e59d
Compare
hessjcg
force-pushed
the
gh-2256-fuse-exit
branch
from
c44e59d
to
ebb71af
Compare
enocom
changed the title
internal/proxy/proxy_other.go
Outdated
| <-ctx.Done() | ||
| return ctx.Err() | ||
| select { | ||
| case cErr := <-c.fuseExitCh: |
There was a problem hiding this comment.
nit: since there are no other errors in scope here, err would be fine here.
There was a problem hiding this comment.
Updated to reuse err.
| conn := tryDialUnix(t, socketPath) | ||
| defer conn.Close() | ||
|
|
||
| var got []string |
There was a problem hiding this comment.
Is this block of assertions necessary? tryDialUnix should wait until a connection has been established and we have tests elsewhere that ensure the wiring is correct (see above for example).
There was a problem hiding this comment.
I removed the d.dialedInstances() assertion.
internal/proxy/fuse_test.go
Outdated
| servErrCh := make(chan error) | ||
| go func() { | ||
| servErr := c.Serve(context.Background(), func() { close(ready) }) | ||
| servErrCh <- servErr |
There was a problem hiding this comment.
This will leak goroutines if servErrCh is full.
How about:
select {
case servErrCh <- servErr:
default:
// exit background thread
}
hessjcg
force-pushed
the
gh-2256-fuse-exit
branch
2 times, most recently
from
5ef7f29
to
a46c983
Compare
internal/proxy/fuse_test.go
Outdated
| go c.Serve(context.Background(), func() { close(ready) }) | ||
| servErrCh := make(chan error) | ||
| go func() { | ||
| servErr := c.Serve(context.Background(), func() { close(ready) }) |
There was a problem hiding this comment.
My suggestion seems to have broken your test. Maybe it's better to block on the error channel or a context that you can cancel at the end of the test run.
There was a problem hiding this comment.
There must be some subtle timing difference in the ubuntu implementation of channels. I put the test back to how it was and now it passes. I think its OK for this goroutine to block until servErrCh can accept an error.
There was a problem hiding this comment.
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
go func() {
servErr := c.Server(ctx, func() { close(ready) })
select {
case servErrCh <- serrErr:
case <-ctx.Done():
}
}()
hessjcg
force-pushed
the
gh-2256-fuse-exit
branch
8 times, most recently
from
3b2b3b5
to
1584076
Compare
hessjcg
force-pushed
the
gh-2256-fuse-exit
branch
from
1584076
to
14352f8
Compare
|
|
||
| ctx, cancel := context.WithCancel(context.Background()) | ||
| go func() { | ||
|
|
This change makes FUSE-configured connections behave the same way as normal connections:
the proxy process should exit when there is an error accepting a connection while listening on
the local server socket.
Fixes #2256