diff --git a/src/core/ext/transport/chttp2/transport/context_list.cc b/src/core/ext/transport/chttp2/transport/context_list.cc index df09809067d..245e8135833 100644 --- a/src/core/ext/transport/chttp2/transport/context_list.cc +++ b/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(head->byte_offset_); + if (ts) { + ts->byte_offset = static_cast(head->byte_offset_); + } write_timestamps_callback_g(head->trace_context_, ts, error); } to_be_freed = head; diff --git a/test/core/iomgr/buffer_list_test.cc b/test/core/iomgr/buffer_list_test.cc index 70e36940425..d71bb9ace2b 100644 --- a/test/core/iomgr/buffer_list_test.cc +++ b/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(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(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) { diff --git a/test/core/transport/chttp2/context_list_test.cc b/test/core/transport/chttp2/context_list_test.cc index 0379eaaee4c..f8c0bee0afb 100644 --- a/test/core/transport/chttp2/context_list_test.cc +++ b/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(arg); gpr_atm_rel_store(done, static_cast(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 s; + s.reserve(kNumElems); + gpr_atm verifier_called[kNumElems]; + for (auto i = 0; i < kNumElems; i++) { + s.push_back(static_cast( + gpr_malloc(grpc_transport_stream_size(t)))); + grpc_transport_init_stream(reinterpret_cast(t), + reinterpret_cast(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(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(1)); + grpc_transport_destroy_stream(reinterpret_cast(t), + reinterpret_cast(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