Skip to content

Commit

Permalink
[ObjC] do not set cfstream client to null on shutdown (grpc#36415)
Browse files Browse the repository at this point in the history
Duo crashes occasionally with the stack trace bellow. I wasn't able to reproduce with stress run a modified version of existing tests, however I suspect what happened was that:

cfstream endpoint was shutdown while a callback is being scheduled, so

  a. [retain](https://github.com/opensource-apple/CF/blob/master/CFStream.c#L595-L598) is about to be called after null check
  b. client callback [context is cleared](https://github.com/opensource-apple/CF/blob/master/CFStream.c#L1322) during shutdown

Since there is no lock in (b), a race can happen and result in retain to be a null pointer and caused the crash.
This PR removed setting the client callback to null so the ongoing event delivery can continue; and added clear the dispatch queue so less likely events are scheduled while connection is shutting down.

----
Crash stack trace:

```
Thread 1 (id: 0x0025a274)CRASHED
Exception infoEXC_BAD_ACCESS / KERN_INVALID_ADDRESS @0x00000008
Stack Quality95%Show frame trust levels
0x00000001034e79a4    (Tachyon -atomic:1014)        long std::__1::__cxx_atomic_fetch_add[abi:v160006]<long>(std::__1::__cxx_atomic_base_impl<long>*, long, std::__1::memory_order)
0x00000001034e79a4    (Tachyon -atomic:1649)        std::__1::__atomic_base<long, true>::fetch_add[abi:v160006](long, std::__1::memory_order)
0x00000001034e79a4    (Tachyon -ref_counted.h:78)        grpc_core::RefCount::Ref(long)
0x00000001034e79a4    (Tachyon -ref_counted.h:379)        grpc_core::RefCounted<grpc_event_engine::experimental::CFStreamEndpointImpl, grpc_core::PolymorphicRefCount, grpc_core::UnrefDelete>::IncrementRefCount() const
0x00000001034e79a4    (Tachyon -ref_counted.h:288)        grpc_core::RefCounted<grpc_event_engine::experimental::CFStreamEndpointImpl, grpc_core::PolymorphicRefCount, grpc_core::UnrefDelete>::Ref()
0x00000001034e79a4    (Tachyon -cfstream_endpoint.h:75)        grpc_event_engine::experimental::CFStreamEndpointImpl::Retain(void*)
0x0000000199d2a814    (CoreFoundation + 0x000b8814)        _signalEventSync
0x0000000199db0654    (CoreFoundation + 0x0013e654)        ___signalEventQueue_block_invoke
0x00000001a1b6d138    (libdispatch.dylib + 0x00002138)        _dispatch_call_block_and_release
0x00000001a1b6edd0    (libdispatch.dylib + 0x00003dd0)        _dispatch_client_callout
0x00000001a1b71f68    (libdispatch.dylib + 0x00006f68)        _dispatch_queue_override_invoke
0x00000001a1b80890    (libdispatch.dylib + 0x00015890)        _dispatch_root_queue_drain
0x00000001a1b81098    (libdispatch.dylib + 0x00016098)        _dispatch_worker_thread2
0x00000001f5c54ee0    (libsystem_pthread.dylib + 0x00001ee0)        _pthread_wqthread
```

Closes grpc#36415

COPYBARA_INTEGRATE_REVIEW=grpc#36415 from HannahShiSFB:cf-event-engine-crash f8f609f
PiperOrigin-RevId: 627154676
  • Loading branch information
HannahShiSFB authored and copybara-github committed Apr 22, 2024
1 parent 297e22c commit 10a95e0
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions src/core/lib/event_engine/cf_engine/cfstream_endpoint.cc
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,9 @@ void CFStreamEndpointImpl::Shutdown() {
read_event_.SetShutdown(shutdownStatus);
write_event_.SetShutdown(shutdownStatus);

CFReadStreamSetClient(cf_read_stream_, kCFStreamEventNone, nullptr, nullptr);
CFWriteStreamSetClient(cf_write_stream_, kCFStreamEventNone, nullptr,
nullptr);
CFReadStreamSetDispatchQueue(cf_read_stream_, nullptr);
CFWriteStreamSetDispatchQueue(cf_write_stream_, nullptr);

CFReadStreamClose(cf_read_stream_);
CFWriteStreamClose(cf_write_stream_);
}
Expand Down

0 comments on commit 10a95e0

Please sign in to comment.