Fixed bug in cronet_transport where completed ops were not removed fr…

…om storage
grpc · Oct 3, 2019 · 4581fe0 · 4581fe0
1 parent 04495e5
commit 4581fe0
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 15 deletions.
diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.cc b/src/core/ext/transport/cronet/transport/cronet_transport.cc
@@ -1348,21 +1348,26 @@ static enum e_op_result execute_stream_op(struct op_and_state* oas) {
       stream_state->cancel_error =
           GRPC_ERROR_REF(stream_op->payload->cancel_stream.cancel_error);
     }
-  } else if (stream_op->on_complete &&
-             op_can_be_run(stream_op, s, &oas->state, OP_ON_COMPLETE)) {
+  } else if (op_can_be_run(stream_op, s, &oas->state, OP_ON_COMPLETE)) {
     CRONET_LOG(GPR_DEBUG, "running: %p  OP_ON_COMPLETE", oas);
     if (stream_state->state_op_done[OP_CANCEL_ERROR]) {
-      GRPC_CLOSURE_SCHED(stream_op->on_complete,
-                         GRPC_ERROR_REF(stream_state->cancel_error));
+      if (stream_op->on_complete) {
+        GRPC_CLOSURE_SCHED(stream_op->on_complete,
+                           GRPC_ERROR_REF(stream_state->cancel_error));
+      }
     } else if (stream_state->state_callback_received[OP_FAILED]) {
-      GRPC_CLOSURE_SCHED(
-          stream_op->on_complete,
-          make_error_with_desc(GRPC_STATUS_UNAVAILABLE, "Unavailable."));
+      if (stream_op->on_complete) {
+        GRPC_CLOSURE_SCHED(
+            stream_op->on_complete,
+            make_error_with_desc(GRPC_STATUS_UNAVAILABLE, "Unavailable."));
+      }
     } else {
       /* All actions in this stream_op are complete. Call the on_complete
        * callback
        */
-      GRPC_CLOSURE_SCHED(stream_op->on_complete, GRPC_ERROR_NONE);
+      if (stream_op->on_complete) {
+        GRPC_CLOSURE_SCHED(stream_op->on_complete, GRPC_ERROR_NONE);
+      }
     }
     oas->state.state_op_done[OP_ON_COMPLETE] = true;
     oas->done = true;

diff --git a/src/objective-c/tests/ConfigureCronet.m b/src/objective-c/tests/ConfigureCronet.m
@@ -30,6 +30,5 @@ void configureCronet(void) {
                                                          inDomains:NSUserDomainMask] lastObject];
     NSLog(@"Documents directory: %@", url);
     [Cronet start];
-    [Cronet startNetLogToFile:@"cronet_netlog.json" logBytes:YES];
   });
 }
diff --git a/src/objective-c/tests/PerfTests/PerfTests.m b/src/objective-c/tests/PerfTests/PerfTests.m
@@ -143,8 +143,8 @@ - (void)pingPongV2APIWithRequest:(RMTStreamingOutputCallRequest *)request
                                             messageCallback:^(id message) {
                                               int indexCopy;
                                               @synchronized(self) {
-                                                index += 1;
                                                 indexCopy = index;
+                                                index += 1;
                                               }
                                               if (indexCopy < numMessages) {
                                                 [call writeMessage:request];
@@ -181,7 +181,7 @@ - (void)testPingPongRPCWithV2API {
   [self pingPongV2APIWithRequest:request numMessages:1000 options:options];
 
   [self measureBlock:^{
-    [self pingPongV2APIWithRequest:request numMessages:10000 options:options];
+    [self pingPongV2APIWithRequest:request numMessages:1000 options:options];
   }];
 }
 
@@ -198,7 +198,7 @@ - (void)testPingPongRPCWithFlowControl {
   [self pingPongV2APIWithRequest:request numMessages:1000 options:options];
 
   [self measureBlock:^{
-    [self pingPongV2APIWithRequest:request numMessages:10000 options:options];
+    [self pingPongV2APIWithRequest:request numMessages:1000 options:options];
   }];
 }
 
@@ -215,7 +215,7 @@ - (void)testPingPongRPCWithInterceptor {
   [self pingPongV2APIWithRequest:request numMessages:1000 options:options];
 
   [self measureBlock:^{
-    [self pingPongV2APIWithRequest:request numMessages:10000 options:options];
+    [self pingPongV2APIWithRequest:request numMessages:1000 options:options];
   }];
 }
 
@@ -254,7 +254,7 @@ - (void)testPingPongRPCWithV1API {
   id request = [RMTStreamingOutputCallRequest messageWithPayloadSize:@1 requestedResponseSize:@1];
   [self pingPongV1APIWithRequest:request numMessages:1000];
   [self measureBlock:^{
-    [self pingPongV1APIWithRequest:request numMessages:10000];
+    [self pingPongV1APIWithRequest:request numMessages:1000];
   }];
 }
 
@@ -298,7 +298,7 @@ - (void)unaryRPCsWithServices:(NSArray<RMTTestService *> *)services
   [self waitForExpectationsWithTimeout:TEST_TIMEOUT handler:nil];
 }
 
-- (void)testUnaryRPC {
+- (void)test1MBUnaryRPC {
   // Workaround Apple CFStream bug
   if ([self isUsingCFStream]) {
     return;
@@ -329,6 +329,32 @@ - (void)testUnaryRPC {
   }];
 }
 
+- (void)test1KBUnaryRPC {
+  RMTSimpleRequest *request = [RMTSimpleRequest message];
+  request.responseSize = 1024;
+  request.payload.body = [NSMutableData dataWithLength:1024];
+
+  GRPCMutableCallOptions *options = [[GRPCMutableCallOptions alloc] init];
+  options.transport = [[self class] transport];
+  options.PEMRootCertificates = [[self class] PEMRootCertificates];
+  options.hostNameOverride = [[self class] hostNameOverride];
+
+  // warm up
+  [self unaryRPCsWithServices:@[ self->_service ]
+                      request:request
+              callsPerService:1000
+          maxOutstandingCalls:100
+                  callOptions:options];
+
+  [self measureBlock:^{
+    [self unaryRPCsWithServices:@[ self->_service ]
+                        request:request
+                callsPerService:1000
+            maxOutstandingCalls:100
+                    callOptions:options];
+  }];
+}
+
 - (void)testMultipleChannels {
   NSString *port = [[[self class] host] componentsSeparatedByString:@":"][1];
   int kNumAddrs = 10;