Skip to content

Commit

Permalink
synchronize stream creation
Browse files Browse the repository at this point in the history
SPDY implementations MUST synchronize stream creation (all the way
to writing the frame) as stream IDs **MUST** increase monotonically.
There was already synchronization on ID creation, but if CreateStream
was called from different goroutines, the calls could interleave
before writing the actual frames to the wire, getting out of order.
(This is _incorrect_ and should cause the other side to STREAM_RST.)

We fix this by moving the s.nextIdLock.Lock() out into the
CreateStream call itself. This way, the frame will be written
before letting the next stream grab the ID and write the frame.
There are probably better ways of doing this, but this minimizes
changes.

License: Apache 2
Signed-off-by: Juan Batiz-Benet <[email protected]>
  • Loading branch information
jbenet committed Jun 15, 2015
1 parent e372247 commit f8ff317
Showing 1 changed file with 5 additions and 2 deletions.
7 changes: 5 additions & 2 deletions connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,11 @@ func (s *Connection) remoteStreamFinish(stream *Stream) {
// the stream Wait or WaitTimeout function on the stream returned
// by this function.
func (s *Connection) CreateStream(headers http.Header, parent *Stream, fin bool) (*Stream, error) {
// MUST synchronize stream creation (all the way to writing the frame)
// as stream IDs **MUST** increase monotonically.
s.nextIdLock.Lock()
defer s.nextIdLock.Unlock()

streamId := s.getNextStreamId()
if streamId == 0 {
return nil, fmt.Errorf("Unable to get new stream id")
Expand Down Expand Up @@ -833,8 +838,6 @@ func (s *Connection) sendStream(stream *Stream, fin bool) error {
// getNextStreamId returns the next sequential id
// every call should produce a unique value or an error
func (s *Connection) getNextStreamId() spdy.StreamId {
s.nextIdLock.Lock()
defer s.nextIdLock.Unlock()
sid := s.nextStreamId
if sid > 0x7fffffff {
return 0
Expand Down

0 comments on commit f8ff317

Please sign in to comment.