[GitHub] [pulsar] BewareMyPower opened a new pull request, #15316: [C++] Wait until event loops terminates when closing the Client

[GitBox <gi...@apache.org>]

BewareMyPower opened a new pull request, #15316:
URL: https://github.com/apache/pulsar/pull/15316

   Fixes #13267
   
   ### Motivation
   
   Unlike Java client, the `Client` of C++ client has a `shutdown` method
   that is responsible to execute the following steps:
   1. Call `shutdown` on all internal producers and consumers
   2. Close all connections in the pool
   3. Close all executors of the executor providers.
   
   When an executor is closed, it call `io_service::stop()`, which makes
   the event loop (`io_service::run()`) in another thread return as soon as
   possible. However, there is no wait operation. If a client failed to
   create a producer or consumer, the `close` method will call `shutdown`
   and close all executors immediately and exits the application. In this
   case, the detached event loop thread might not exit ASAP, then valgrind
   will detect the memory leak.
   
   This memory leak can be avoided by sleeping for a while after
   `Client::close` returns or there are still other things to do after
   that. However, we should still adopt the semantics that after
   `Client::shutdown` returns, all event loop threads should be terminated.
   
   ### Modifications
   - Add a timeout parameter to the `close` method of `ExecutorService` and
     `ExecutorServiceProvider` as the max blocking timeout if it's
     non-negative.
   - Add a `TimeoutProcessor` helper class to update the left timeout after
     calling all methods that accept the timeout parameter.
   - Call `close` on all `ExecutorServiceProvider`s in
     `ClientImpl::shutdown` with 500ms timeout, which could be long enough.
     In addition, in `handleClose` method, call `shutdown` in another
     thread to avoid the deadlock.
   
   ### Verifying this change
   
   After applying this patch, the reproduce code in #13627 will pass the
   valgrind check.
   
   ```
   ==3013== LEAK SUMMARY:
   ==3013==    definitely lost: 0 bytes in 0 blocks
   ==3013==    indirectly lost: 0 bytes in 0 blocks
   ==3013==      possibly lost: 0 bytes in 0 blocks
   ```
   
   ### Documentation
   
   Check the box below or label this PR directly.
   
   Need to update docs? 
   
   - [ ] `doc-required` 
   (Your PR needs to update docs and you will update later)
     
   - [x] `no-need-doc` 
   (Please explain why)
     
   - [ ] `doc` 
   (Your PR contains doc changes)
   
   - [ ] `doc-added`
   (Docs have been already added)


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar] BewareMyPower merged pull request #15316: [C++] Wait until event loop terminates when closing the Client

[GitBox <gi...@apache.org>]

BewareMyPower merged PR #15316:
URL: https://github.com/apache/pulsar/pull/15316


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org