Add a few tests for ContextList and BufferList. Also, check that

timestamps is non-null before modifying it.

src/core/ext/transport/chttp2/transport/context_list.cc

@@ -46,7 +46,9 @@ void ContextList::Execute(void* arg, grpc_core::Timestamps* ts,
   ContextList* to_be_freed;
   while (head != nullptr) {
     if (write_timestamps_callback_g) {
-      ts->byte_offset = static_cast<uint32_t>(head->byte_offset_);
+      if (ts) {
+        ts->byte_offset = static_cast<uint32_t>(head->byte_offset_);
+      }
       write_timestamps_callback_g(head->trace_context_, ts, error);
     }
     to_be_freed = head;

test/core/iomgr/buffer_list_test.cc

@@ -92,9 +92,33 @@ static void TestVerifierCalledOnAck() {
   grpc_core::TracedBuffer::Shutdown(&list, nullptr, GRPC_ERROR_NONE);
 }
 
+/** Tests that shutdown can be called repeatedly.
+ */
+static void TestRepeatedShutdown() {
+  struct sock_extended_err serr;
+  serr.ee_data = 213;
+  serr.ee_info = grpc_core::SCM_TSTAMP_ACK;
+  struct grpc_core::scm_timestamping tss;
+  tss.ts[0].tv_sec = 123;
+  tss.ts[0].tv_nsec = 456;
+  grpc_core::grpc_tcp_set_write_timestamps_callback(
+      TestVerifierCalledOnAckVerifier);
+  grpc_core::TracedBuffer* list = nullptr;
+  gpr_atm verifier_called;
+  gpr_atm_rel_store(&verifier_called, static_cast<gpr_atm>(0));
+  grpc_core::TracedBuffer::AddNewEntry(&list, 213, 0, &verifier_called);
+  grpc_core::TracedBuffer::ProcessTimestamp(&list, &serr, nullptr, &tss);
+  GPR_ASSERT(gpr_atm_acq_load(&verifier_called) == static_cast<gpr_atm>(1));
+  GPR_ASSERT(list == nullptr);
+  grpc_core::TracedBuffer::Shutdown(&list, nullptr, GRPC_ERROR_NONE);
+  grpc_core::TracedBuffer::Shutdown(&list, nullptr, GRPC_ERROR_NONE);
+  grpc_core::TracedBuffer::Shutdown(&list, nullptr, GRPC_ERROR_NONE);
+}
+
 static void TestTcpBufferList() {
   TestVerifierCalledOnAck();
   TestShutdownFlushesList();
+  TestRepeatedShutdown();
 }
 
 int main(int argc, char** argv) {

test/core/transport/chttp2/context_list_test.cc

@@ -42,21 +42,29 @@ void TestExecuteFlushesListVerifier(void* arg, grpc_core::Timestamps* ts,
                                     grpc_error* error) {
   ASSERT_NE(arg, nullptr);
   EXPECT_EQ(error, GRPC_ERROR_NONE);
-  EXPECT_EQ(ts->byte_offset, kByteOffset);
+  if (ts) {
+    EXPECT_EQ(ts->byte_offset, kByteOffset);
+  }
   gpr_atm* done = reinterpret_cast<gpr_atm*>(arg);
   gpr_atm_rel_store(done, static_cast<gpr_atm>(1));
 }
 
 void discard_write(grpc_slice slice) {}
 
+class ContextListTest : public ::testing::Test {
+ protected:
+  void SetUp() override {
+    grpc_http2_set_write_timestamps_callback(TestExecuteFlushesListVerifier);
+    grpc_http2_set_fn_get_copied_context(DummyArgsCopier);
+  }
+};
+
 /** Tests that all ContextList elements in the list are flushed out on
  * execute.
  * Also tests that arg and byte_counter are passed correctly.
  */
-TEST(ContextList, ExecuteFlushesList) {
+TEST_F(ContextListTest, ExecuteFlushesList) {
   grpc_core::ContextList* list = nullptr;
-  grpc_http2_set_write_timestamps_callback(TestExecuteFlushesListVerifier);
-  grpc_http2_set_fn_get_copied_context(DummyArgsCopier);
   const int kNumElems = 5;
   grpc_core::ExecCtx exec_ctx;
   grpc_stream_refcount ref;
@@ -95,6 +103,62 @@ TEST(ContextList, ExecuteFlushesList) {
   grpc_resource_quota_unref(resource_quota);
   exec_ctx.Flush();
 }
+
+TEST_F(ContextListTest, EmptyList) {
+  grpc_core::ContextList* list = nullptr;
+  grpc_core::ExecCtx exec_ctx;
+  grpc_core::Timestamps ts;
+  grpc_core::ContextList::Execute(list, &ts, GRPC_ERROR_NONE);
+  exec_ctx.Flush();
+}
+
+TEST_F(ContextListTest, EmptyListEmptyTimestamp) {
+  grpc_core::ContextList* list = nullptr;
+  grpc_core::ExecCtx exec_ctx;
+  grpc_core::ContextList::Execute(list, nullptr, GRPC_ERROR_NONE);
+  exec_ctx.Flush();
+}
+
+TEST_F(ContextListTest, NonEmptyListEmptyTimestamp) {
+  grpc_core::ContextList* list = nullptr;
+  const int kNumElems = 5;
+  grpc_core::ExecCtx exec_ctx;
+  grpc_stream_refcount ref;
+  GRPC_STREAM_REF_INIT(&ref, 1, nullptr, nullptr, "dummy ref");
+  grpc_resource_quota* resource_quota =
+      grpc_resource_quota_create("context_list_test");
+  grpc_endpoint* mock_endpoint =
+      grpc_mock_endpoint_create(discard_write, resource_quota);
+  grpc_transport* t =
+      grpc_create_chttp2_transport(nullptr, mock_endpoint, true);
+  std::vector<grpc_chttp2_stream*> s;
+  s.reserve(kNumElems);
+  gpr_atm verifier_called[kNumElems];
+  for (auto i = 0; i < kNumElems; i++) {
+    s.push_back(static_cast<grpc_chttp2_stream*>(
+        gpr_malloc(grpc_transport_stream_size(t))));
+    grpc_transport_init_stream(reinterpret_cast<grpc_transport*>(t),
+                               reinterpret_cast<grpc_stream*>(s[i]), &ref,
+                               nullptr, nullptr);
+    s[i]->context = &verifier_called[i];
+    s[i]->byte_counter = kByteOffset;
+    gpr_atm_rel_store(&verifier_called[i], static_cast<gpr_atm>(0));
+    grpc_core::ContextList::Append(&list, s[i]);
+  }
+  grpc_core::ContextList::Execute(list, nullptr, GRPC_ERROR_NONE);
+  for (auto i = 0; i < kNumElems; i++) {
+    EXPECT_EQ(gpr_atm_acq_load(&verifier_called[i]), static_cast<gpr_atm>(1));
+    grpc_transport_destroy_stream(reinterpret_cast<grpc_transport*>(t),
+                                  reinterpret_cast<grpc_stream*>(s[i]),
+                                  nullptr);
+    exec_ctx.Flush();
+    gpr_free(s[i]);
+  }
+  grpc_transport_destroy(t);
+  grpc_resource_quota_unref(resource_quota);
+  exec_ctx.Flush();
+}
+
 }  // namespace
 }  // namespace testing
 }  // namespace grpc_core