Make sure mountoptions.{Send, Recv} respects ctx's deadline
#345
Conversation
Signed-off-by: Burak Varlı <[email protected]>
|
For my own understanding, why do we care about deadlines, and what are they used for? |
| if err != nil { | ||
| return fmt.Errorf("failed to dial to unix socket %s: %w", sockPath, err) | ||
| } | ||
| defer conn.Close() | ||
|
|
||
| unixConn := conn.(*net.UnixConn) | ||
|
|
||
| // `unixConn.WriteMsgUnix` does not respect `ctx`'s deadline, we need to call `unixConn.SetDeadline` to ensure `unixConn.WriteMsgUnix` has a deadline. | ||
| if deadline, ok := ctx.Deadline(); ok { |
There was a problem hiding this comment.
Can ctx's deadline change, and if so is this mirrored automatically? Does it matter?
There was a problem hiding this comment.
This ctx is immutable basically, you can create a new context from this one with a new timeout via context.WithTimeout(ctx, newTimeout) but this would be a different context. Here we just respect the deadline/timeout passed to this function on the original call.
It's for timeouts, usually contexts are used to indicate timeouts on the operations. For example, before calling But without this change, that timeout is ignored basically, and that function might block forever until it's cancelled on a higher level somehow |
| if err != nil { | ||
| return Options{}, fmt.Errorf("failed to listen unix socket %s: %w", sockPath, err) | ||
| } | ||
| defer l.Close() | ||
|
|
||
| // `l.Accept` does not respect `ctx`'s deadline, we need to call `ul.SetDeadline` to ensure `l.Accept` has a deadline. | ||
| if deadline, ok := ctx.Deadline(); ok { |
There was a problem hiding this comment.
Is it useful to make this a function? It's repeated twice here and I'm not sure if we'd want to use this more often
There was a problem hiding this comment.
I don't expect we'd need this function somewhere else, but I can extract it into a private function in this package.
There was a problem hiding this comment.
So seems like these are two different types in two functions, net.UnixListener and net.UnixConn and they don't share any common interface. So we need to define an interface for SetDeadline method to use it with both types. Not sure if this is better or create confusion:
type deadline interface {
SetDeadline(t time.Time) error
}
func populateDeadlineFromContext(ctx context.Context, conn deadline) error {
if deadline, ok := ctx.Deadline(); ok {
return conn.SetDeadline(deadline)
}
return nil
}
net.{Listen, Dial}functions does not accept context argument, but only{ListenContext, DialContext}counterparts do. But even those functions does not respect to context's deadline, so we need to callSetDeadlinewith passed context's deadline explicitly to ensure context's deadline is respected.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.