Parcourir la source

Make sure we don't write additional DATA frames when flushing once request is finished (#19)

## Motivation
When a client sends a request, to signal its end to the server, it has
to send an empty DATA frame with END_STREAM set.
In the current implementation, the state machine correctly signals the
handler that there are no more messages to be encoded, and so the client
proceeds to write the empty DATA frame with EOS set when flushing.
However, if subsequent flushes are triggered down the pipeline, the
client will keep writing these empty DATA frames, which is a bug.

## Modifications
Make sure that we only write the empty DATA frame once. If we've already
done it, the client can ignore subsequent flushes, as it is done.
Gus Cairo il y a 1 an
Parent
commit
5cd9f91152

+ 7 - 1
Sources/GRPCNIOTransportCore/Client/GRPCClientStreamHandler.swift

@@ -29,6 +29,7 @@ final class GRPCClientStreamHandler: ChannelDuplexHandler {
 
   private var isReading = false
   private var flushPending = false
+  private var requestFinished = false
 
   init(
     methodDescriptor: MethodDescriptor,
@@ -263,6 +264,12 @@ extension GRPCClientStreamHandler {
           )
 
         case .noMoreMessages:
+          // If we're done writing (i.e. we have no more messages returned from state
+          // machine) then return.
+          if self.requestFinished { return }
+
+          self.requestFinished = true
+
           // Write an empty data frame with the EOS flag set, to signal the RPC
           // request is now finished.
           context.write(
@@ -276,7 +283,6 @@ extension GRPCClientStreamHandler {
             ),
             promise: nil
           )
-
           context.flush()
           break loop
 

+ 5 - 3
Tests/GRPCNIOTransportCoreTests/Client/GRPCClientStreamHandlerTests.swift

@@ -529,14 +529,16 @@ final class GRPCClientStreamHandlerTests: XCTestCase {
     // Half-close the outbound end: this would be triggered by finishing the client's writer.
     XCTAssertNoThrow(channel.close(mode: .output, promise: nil))
 
-    // Flush to make sure the EOS is written.
-    channel.flush()
-
     // Make sure the EOS frame was sent
     let emptyEOSFrame = try channel.assertReadDataOutbound()
     XCTAssertEqual(emptyEOSFrame.data, .byteBuffer(.init()))
     XCTAssertTrue(emptyEOSFrame.endStream)
 
+    // Make sure that, if we flush again, we're not writing anything else down
+    // the stream. We should have closed at this point.
+    channel.flush()
+    XCTAssertNil(try channel.readOutbound(as: HTTP2Frame.FramePayload.self))
+
     // Make sure we cannot write anymore because client's closed.
     XCTAssertThrowsError(
       ofType: RPCError.self,