Copilot commented on code in PR #3590:
URL: https://github.com/apache/brpc/pull/3590#discussion_r4305665949


##########
src/brpc/server.cpp:
##########
@@ -1370,6 +1359,34 @@ int Server::Join() {
     return 0;
 }
 
+void Server::DestroyPreStartAllocations() {
+    if (_status == RUNNING || _status == STOPPING) {
+        // Handled by Join().
+        return;
+    }
+    DestroyThreadLocalResources();
+}
+
+void Server::DestroyThreadLocalResources() {
+    if (_keytable_pool != nullptr) {
+        // Keep the TLS key valid until its pooled data has been destroyed.
+        // Deleting the key first would disable DestroyServerTLS().
+        CHECK_EQ(0, bthread_keytable_pool_destroy(_keytable_pool));
+        // Don't delete the pool struct. Bthreads created with
+        // attr.keytable_pool == _keytable_pool may outlive shutdown or
+        // a failed start and still call return_keytable() on exit. A
+        // destroyed pool makes them delete their KeyTables directly, but the
+        // struct itself must stay valid. The leak is annotated in
+        // StartInternal().
+        _keytable_pool = nullptr;
+    }
+    // Delete tls_key created in StartInternal().
+    if (_tl_options.tls_key != INVALID_BTHREAD_KEY) {
+        CHECK_EQ(0, bthread_key_delete(_tl_options.tls_key));
+        _tl_options.tls_key = INVALID_BTHREAD_KEY;

Review Comment:
   Deleting the TLS key here can leak data in a bthread created by 
`bthread_init_fn` that outlives the failed `Start()`. That bthread's keytable 
is not destroyed by `bthread_keytable_pool_destroy`; when it is later returned, 
`KeyTable` sees the key's version has changed and skips `DestroyServerTLS`, so 
data created by the server's `DataFactory` is never released. Defer key 
deletion until all bthreads using this pool have exited, or explicitly drain 
those keytables before invalidating the key.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to