Merge pull request #2500 from lucas-clemente/no-retransmission-after-cancelation

don't dequeue retransmitted STREAM frames after the stream was canceled

packet_packer.go

@@ -565,7 +565,7 @@ func (p *packetPacker) MaybePackProbePacket(encLevel protocol.EncryptionLevel) (
 	default:
 		panic("unknown encryption level")
 	}
-	if err != nil {
+	if err != nil || contents == nil {
 		return nil, err
 	}
 	if p.perspective == protocol.PerspectiveClient && encLevel == protocol.EncryptionInitial {

packet_packer_test.go

@@ -1127,6 +1127,18 @@ var _ = Describe("Packet packer", func() {
 				Expect(packet.frames).To(HaveLen(1))
 				Expect(packet.frames[0].Frame).To(Equal(f))
 			})
+
+			It("returns nil if there's no probe data to send", func() {
+				sealingManager.EXPECT().Get1RTTSealer().Return(getSealer(), nil)
+				ackFramer.EXPECT().GetAckFrame(protocol.Encryption1RTT)
+				pnManager.EXPECT().PeekPacketNumber(protocol.Encryption1RTT).Return(protocol.PacketNumber(0x42), protocol.PacketNumberLen2)
+				expectAppendControlFrames()
+				expectAppendStreamFrames()
+
+				packet, err := packer.MaybePackProbePacket(protocol.Encryption1RTT)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(packet).To(BeNil())
+			})
 		})
 	})
 })

send_stream.go

@@ -166,6 +166,10 @@ func (s *sendStream) popStreamFrame(maxBytes protocol.ByteCount) (*ackhandler.Fr
 }
 
 func (s *sendStream) popNewOrRetransmittedStreamFrame(maxBytes protocol.ByteCount) (*wire.StreamFrame, bool /* has more data to send */) {
+	if s.canceledWrite || s.closeForShutdownErr != nil {
+		return nil, false
+	}
+
 	if len(s.retransmissionQueue) > 0 {
 		f, hasMoreRetransmissions := s.maybeGetRetransmission(maxBytes)
 		if f != nil || hasMoreRetransmissions {
@@ -195,10 +199,6 @@ func (s *sendStream) popNewOrRetransmittedStreamFrame(maxBytes protocol.ByteCoun
 }
 
 func (s *sendStream) popNewStreamFrame(f *wire.StreamFrame, maxBytes protocol.ByteCount) bool {
-	if s.canceledWrite || s.closeForShutdownErr != nil {
-		return false
-	}
-
 	maxDataLen := f.MaxDataLen(maxBytes, s.version)
 	if maxDataLen == 0 { // a STREAM frame must have at least one byte of data
 		return s.dataForWriting != nil
@@ -329,7 +329,6 @@ func (s *sendStream) Close() error {
 
 func (s *sendStream) CancelWrite(errorCode protocol.ApplicationErrorCode) {
 	s.cancelWriteImpl(errorCode, fmt.Errorf("Write on stream %d canceled with error code %d", s.streamID, errorCode))
-
 }
 
 // must be called after locking the mutex

send_stream_test.go

@@ -755,6 +755,22 @@ var _ = Describe("Send Stream", func() {
 			Expect(newFrame).ToNot(BeNil())
 			Expect(newFrame.Frame.(*wire.StreamFrame).Data).To(Equal([]byte("foobar")))
 		})
+
+		It("doesn't get a retransmission after a stream was canceled", func() {
+			str.numOutstandingFrames = 1
+			f := &wire.StreamFrame{
+				Data:           []byte("foobar"),
+				Offset:         0x42,
+				DataLenPresent: false,
+			}
+			mockSender.EXPECT().onHasStreamData(streamID)
+			str.queueRetransmission(f)
+			mockSender.EXPECT().queueControlFrame(gomock.Any())
+			str.CancelWrite(0)
+			frame, hasMoreData := str.popStreamFrame(protocol.MaxByteCount)
+			Expect(hasMoreData).To(BeFalse())
+			Expect(frame).To(BeNil())
+		})
 	})
 
 	Context("determining when a stream is completed", func() {

session.go

@@ -1404,7 +1404,7 @@ func (s *session) sendProbePacket(encLevel protocol.EncryptionLevel) error {
 			return err
 		}
 	}
-	if packet == nil {
+	if packet == nil || packet.packetContents == nil {
 		return fmt.Errorf("session BUG: couldn't pack %s probe packet", encLevel)
 	}
 	s.sendPackedPacket(packet)