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


##########
src/bthread/task_tracer.cpp:
##########
@@ -435,13 +434,14 @@ TaskTracer::Result TaskTracer::SignalTrace(pthread_t 
worker_tid) {
             if (EINTR == errno) {
                 continue;
             }
+            // Extend the lifetime of `signal_sync' to
+            // avoid it being destroyed in the signal handler.
+            _inuse_signal_syncs.push_back(signal_sync);

Review Comment:
   The SignalSync is only added to `_inuse_signal_syncs` when errno is EINTR, 
but the function returns an error for this condition. This means the failed 
SignalSync will remain in `_inuse_signal_syncs` indefinitely and never be 
cleaned up, causing a memory leak. If the intention is to extend the lifetime 
only for async cleanup, there should be a mechanism to eventually remove it.
   ```suggestion
               // poll failed with error other than EINTR; clean up and return 
error.
               delete signal_sync;
   ```



-- 
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