-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
don't use contexts for deadlines #58
Changes from all commits
05c1acd
6c65037
690685d
030410a
618f6f4
e14ba04
dd5280b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
// Copied from the go standard library. | ||
// | ||
// Copyright 2010 The Go Authors. All rights reserved. | ||
// Use of this source code is governed by a BSD-style | ||
// license that can be found in the LICENSE-BSD file. | ||
|
||
package multiplex | ||
|
||
import ( | ||
"sync" | ||
"time" | ||
) | ||
|
||
// pipeDeadline is an abstraction for handling timeouts. | ||
type pipeDeadline struct { | ||
mu sync.Mutex // Guards timer and cancel | ||
timer *time.Timer | ||
cancel chan struct{} // Must be non-nil | ||
} | ||
|
||
func makePipeDeadline() pipeDeadline { | ||
return pipeDeadline{cancel: make(chan struct{})} | ||
} | ||
|
||
// set sets the point in time when the deadline will time out. | ||
// A timeout event is signaled by closing the channel returned by waiter. | ||
// Once a timeout has occurred, the deadline can be refreshed by specifying a | ||
// t value in the future. | ||
// | ||
// A zero value for t prevents timeout. | ||
func (d *pipeDeadline) set(t time.Time) { | ||
d.mu.Lock() | ||
defer d.mu.Unlock() | ||
|
||
if d.timer != nil && !d.timer.Stop() { | ||
<-d.cancel // Wait for the timer callback to finish and close cancel | ||
} | ||
d.timer = nil | ||
|
||
// Time is zero, then there is no deadline. | ||
closed := isClosedChan(d.cancel) | ||
if t.IsZero() { | ||
if closed { | ||
d.cancel = make(chan struct{}) | ||
} | ||
return | ||
} | ||
|
||
// Time in the future, setup a timer to cancel in the future. | ||
if dur := time.Until(t); dur > 0 { | ||
if closed { | ||
d.cancel = make(chan struct{}) | ||
} | ||
d.timer = time.AfterFunc(dur, func() { | ||
close(d.cancel) | ||
}) | ||
return | ||
} | ||
|
||
// Time in the past, so close immediately. | ||
if !closed { | ||
close(d.cancel) | ||
} | ||
} | ||
|
||
// wait returns a channel that is closed when the deadline is exceeded. | ||
func (d *pipeDeadline) wait() chan struct{} { | ||
d.mu.Lock() | ||
defer d.mu.Unlock() | ||
return d.cancel | ||
} | ||
|
||
func isClosedChan(c <-chan struct{}) bool { | ||
select { | ||
case <-c: | ||
return true | ||
default: | ||
return false | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,9 +38,7 @@ type Stream struct { | |
// for later memory pool freeing | ||
exbuf []byte | ||
|
||
deadlineLock sync.Mutex | ||
wDeadline time.Time | ||
rDeadline time.Time | ||
rDeadline, wDeadline pipeDeadline | ||
|
||
clLock sync.Mutex | ||
closedRemote bool | ||
|
@@ -70,15 +68,7 @@ func (s *Stream) preloadData() { | |
} | ||
} | ||
|
||
func (s *Stream) waitForData(ctx context.Context) error { | ||
s.deadlineLock.Lock() | ||
if !s.rDeadline.IsZero() { | ||
dctx, cancel := context.WithDeadline(ctx, s.rDeadline) | ||
defer cancel() | ||
ctx = dctx | ||
} | ||
s.deadlineLock.Unlock() | ||
|
||
func (s *Stream) waitForData() error { | ||
select { | ||
case <-s.reset: | ||
// This is the only place where it's safe to return these. | ||
|
@@ -91,8 +81,8 @@ func (s *Stream) waitForData(ctx context.Context) error { | |
s.extra = read | ||
s.exbuf = read | ||
return nil | ||
case <-ctx.Done(): | ||
return ctx.Err() | ||
case <-s.rDeadline.wait(): | ||
return errTimeout | ||
} | ||
} | ||
|
||
|
@@ -125,7 +115,7 @@ func (s *Stream) Read(b []byte) (int, error) { | |
default: | ||
} | ||
if s.extra == nil { | ||
err := s.waitForData(context.Background()) | ||
err := s.waitForData() | ||
if err != nil { | ||
return 0, err | ||
} | ||
|
@@ -172,21 +162,7 @@ func (s *Stream) write(b []byte) (int, error) { | |
return 0, errors.New("cannot write to closed stream") | ||
} | ||
|
||
s.deadlineLock.Lock() | ||
wDeadlineCtx, cleanup := func(s *Stream) (context.Context, context.CancelFunc) { | ||
if s.wDeadline.IsZero() { | ||
return s.closedLocal, nil | ||
} else { | ||
return context.WithDeadline(s.closedLocal, s.wDeadline) | ||
} | ||
}(s) | ||
s.deadlineLock.Unlock() | ||
|
||
err := s.mp.sendMsg(wDeadlineCtx, s.id.header(messageTag), b) | ||
|
||
if cleanup != nil { | ||
cleanup() | ||
} | ||
err := s.mp.sendMsg(s.wDeadline.wait(), s.id.header(messageTag), b) | ||
|
||
if err != nil { | ||
if err == context.Canceled { | ||
|
@@ -206,7 +182,7 @@ func (s *Stream) Close() error { | |
ctx, cancel := context.WithTimeout(context.Background(), ResetStreamTimeout) | ||
defer cancel() | ||
|
||
err := s.mp.sendMsg(ctx, s.id.header(closeTag), nil) | ||
err := s.mp.sendMsg(ctx.Done(), s.id.header(closeTag), nil) | ||
|
||
if s.isClosed() { | ||
return nil | ||
|
@@ -219,6 +195,7 @@ func (s *Stream) Close() error { | |
s.doCloseLocal() | ||
|
||
if remote { | ||
s.cancelDeadlines() | ||
s.mp.chLock.Lock() | ||
delete(s.mp.channels, s.id) | ||
s.mp.chLock.Unlock() | ||
|
@@ -252,6 +229,7 @@ func (s *Stream) Reset() error { | |
close(s.reset) | ||
s.doCloseLocal() | ||
s.closedRemote = true | ||
s.cancelDeadlines() | ||
|
||
go s.mp.sendResetMsg(s.id.header(resetTag), true) | ||
|
||
|
@@ -264,24 +242,44 @@ func (s *Stream) Reset() error { | |
return nil | ||
} | ||
|
||
func (s *Stream) cancelDeadlines() { | ||
s.rDeadline.set(time.Time{}) | ||
s.wDeadline.set(time.Time{}) | ||
} | ||
|
||
func (s *Stream) SetDeadline(t time.Time) error { | ||
s.deadlineLock.Lock() | ||
defer s.deadlineLock.Unlock() | ||
s.rDeadline = t | ||
s.wDeadline = t | ||
s.clLock.Lock() | ||
defer s.clLock.Unlock() | ||
|
||
if s.closedRemote || s.isClosed() { | ||
return errStreamClosed | ||
} | ||
|
||
s.rDeadline.set(t) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should try to prevent this from happening after closing the stream so we don't leak a timer. Not that much of an issue but is there some kind of state lock we can take? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could try the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SGTM. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I used the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LGTM. |
||
s.wDeadline.set(t) | ||
return nil | ||
} | ||
|
||
func (s *Stream) SetReadDeadline(t time.Time) error { | ||
s.deadlineLock.Lock() | ||
defer s.deadlineLock.Unlock() | ||
s.rDeadline = t | ||
s.clLock.Lock() | ||
defer s.clLock.Unlock() | ||
|
||
if s.closedRemote { | ||
return errStreamClosed | ||
} | ||
|
||
s.rDeadline.set(t) | ||
return nil | ||
} | ||
|
||
func (s *Stream) SetWriteDeadline(t time.Time) error { | ||
s.deadlineLock.Lock() | ||
defer s.deadlineLock.Unlock() | ||
s.wDeadline = t | ||
s.clLock.Lock() | ||
defer s.clLock.Unlock() | ||
|
||
if s.isClosed() { | ||
return errStreamClosed | ||
} | ||
|
||
s.wDeadline.set(t) | ||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to encapsulate this more... but we can handle that later.