proxy-server: Reject `destroy()` during active calls and calls after `destroy()` #382

pull xyzconstant wants to merge 1 commits into bitcoin-core:master from xyzconstant:2026-10-19-fix-issues-around-destroy changing 6 files +138 −3
  1. xyzconstant commented at 8:43 AM on October 25, 2026: contributor

    Fixes #378

    There are 2 issues with non-libmultiprocess clients:

    1. After a destroy(), any request can still be sent as long as the capability is not dropped. Such request dereferenced a null m_impl pointer (reset by the previous destroy() call) and crashed the server (also reported in bitcoin/bitcoin#36502).
    2. Sending destroy() while another request on the same object was running could free m_impl while that request was still using it.

    The libmultiprocess C++ client cannot trigger either case because it sends destroy() only from the ProxyClient destructor and releases the capability after that.

    This PR implements the fixes suggested in the issue.

    • serverInvoke checks m_destroyed and rejects requests received after destroy() with "method called after destroy".
    • invokeDestroy checks m_active_calls and rejects destroy() with "destroy called while other calls are in progress" when another request on the object is being executed.

    Differences from the suggestion:

    • No mutex is needed because m_active_calls and m_destroyed are accessed only from the event-loop thread.
    • Only requests with an mp.Context are counted in m_active_calls. Requests without one run to completion on the event-loop thread, so destroy() cannot run during their execution.

    Also added a regression test that manually builds and sends requests with Cap'n Proto, reproducing the behavior of a buggy non-libmultiprocess client.

  2. DrahtBot commented at 8:44 AM on October 25, 2026: member

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline and AI policy for information on the review process. A summary of reviews will appear here.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #342 (Allow request cancellation for wrapped C++ methods by xyzconstant)
    • #335 (proxy-io.h: Add Connection disconnect and waitDrained methods by ryanofsky)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  3. xyzconstant force-pushed on Oct 26, 2026
  4. xyzconstant commented at 4:58 PM on October 27, 2026: contributor

    #381 fixes the OpenBSD job failure. I'm working on a fix for the gnu32 one.

    EDIT: Opened #383 to fix the gnu32 failure. It makes shell.nix compile capnproto with GCC too when an i686 platform is detected.

  5. xyzconstant force-pushed on Oct 27, 2026
  6. proxy: Reject destroy() during active calls and calls after destroy()
    A non-libmultiprocess client can send requests after destroy(), causing
    a null m_impl pointer to be dereferenced and triggering a SIGSEGV. It
    can also call destroy() while another request on the same object is
    active, freeing m_impl while that request is using it.
    
    Track requests queued or executing on worker threads with
    m_active_calls. Reject destroy() while other requests are active,
    and reject any requests received after destruction.
    
    Fixes #378
    0e96139da1
  7. xyzconstant force-pushed on Oct 28, 2026

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/libmultiprocess. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-11-02 00:43 UTC

Static mirror of GitHub issue/PR metadata (via 0xB10C github-metadata-backup), served from ryan.int.chaincode.com.