init: Reserve file descriptors for IPC connections #34978

pull enirox001 wants to merge 3 commits into bitcoin:master from enirox001:04-26-ipc-maxconnections changing 3 files +78 −2
  1. enirox001 commented at 8:34 am on April 1, 2026: contributor

    When -ipcbind is used, the node opens one listening socket FD per bound address and accepts concurrent IPC connections. Neither was previously accounted for in min_required_fds, meaning IPC-heavy workloads could silently exhaust available file descriptors.

    This PR adds -ipcmaxconnections (default: 16, mirroring -rpcworkqueue) so operators can control how many FDs are reserved for accepted IPC connections. It also adds ipc_bind to account for the listening socket FDs opened per -ipcbind address, which were previously unaccounted for. A warning is emitted when -ipcmaxconnections is set without -ipcbind.

    Note: this reserves file descriptors at startup. This does not enforce a hard connection limit at the accept loop (this would be addressed in a followup upstream PR in libmultiprocess)

    Suggested by Sjors in #32297 (comment).

  2. DrahtBot commented at 8:35 am on April 1, 2026: contributor

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

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Concept ACK Sjors, kevkevinpal, w0xlt
    Stale ACK ryanofsky

    If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #35037 (ipc: support per-address max-connections options on -ipcbind by enirox001)
    • #17783 (common: Disallow calling IsArgSet() on ALLOW_LIST options by ryanofsky)
    • #17581 (refactor: Remove settings merge reverse precedence code by ryanofsky)
    • #17580 (refactor: Add ALLOW_LIST flags and enforce usage in CheckArgFlags by ryanofsky)
    • #17493 (util: Forbid ambiguous multiple assignments in config file 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.

  3. in src/init.cpp:724 in 90ad4dc5e6
    720@@ -720,6 +721,7 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc)
    721     argsman.AddArg("-server", "Accept command line and JSON-RPC commands", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
    722     if (can_listen_ipc) {
    723         argsman.AddArg("-ipcbind=<address>", "Bind to Unix socket address and listen for incoming connections. Valid address values are \"unix\" to listen on the default path, <datadir>/node.sock, or \"unix:/custom/path\" to specify a custom path. Can be specified multiple times to listen on multiple paths. Default behavior is not to listen on any path. If relative paths are specified, they are interpreted relative to the network data directory. If paths include any parent directory components and the parent directories do not exist, they will be created. Enabling this gives local processes that can access the socket unauthenticated RPC access, so it's important to choose a path with secure permissions if customizing this.", ArgsManager::ALLOW_ANY, OptionsCategory::IPC);
    724+        argsman.AddArg("-ipcmaxconnections=<n>", "Reserve file descriptors for up to <n> concurrent IPC socket connections (default: " + ToString(DEFAULT_IPC_MAX_CONNECTIONS) + ")", ArgsManager::ALLOW_ANY, OptionsCategory::IPC);
    


    Sjors commented at 8:51 am on April 1, 2026:
    Maybe make call this -unixmaxconnections so it’s clear that this covers both the Capnp IPC and the RPC-via-IPC use case (and maybe Tor over unix?).

    enirox001 commented at 3:18 pm on April 1, 2026:
    Done, renamed to -unixmaxconnections.
  4. in src/init.cpp:1036 in 90ad4dc5e6
    1030@@ -1029,16 +1031,34 @@ bool AppInitParameterInteraction(const ArgsManager& args)
    1031 
    1032     // Number of bound interfaces (we have at least one)
    1033     int nBind = std::max(nUserBind, size_t(1));
    1034+
    1035+    // IPC listening socket FDs — one per -ipcbind address, no default listener
    1036+    int nIpcBind = static_cast<int>(args.GetArgs("-ipcbind").size());
    


    Sjors commented at 8:51 am on April 1, 2026:
    New code can use modern variable name style.

    enirox001 commented at 3:19 pm on April 1, 2026:
    Done, renamed to ipc_bind.
  5. Sjors commented at 8:54 am on April 1, 2026: member

    Concept ACK

    This does not enforce a hard connection limit at the accept loop

    I think it should, because the error for running out file descriptors is presumably confusing, and it will happen at unpredictable moments depending on how many RPC, P2P and IPC connections are active.

    It would also be good to cover the number of reserved file descriptors that’s printed in the log. If you do that in a test commit before the actual change, that nicely demonstrates that this PR actually increases it.

  6. DrahtBot added the label CI failed on Apr 1, 2026
  7. enirox001 commented at 2:06 pm on April 1, 2026: contributor

    I think it should, because the error for running out file descriptors is presumably confusing, and it will happen at unpredictable moments depending on how many RPC, P2P and IPC connections are active.

    Regarding the hard connection limit at the accept loop, implementing that would require changes to mp::ListenConnections in libmultiprocess. Would you expect that to be done in this PR, or as a follow-up in libmultiprocess?

  8. Sjors commented at 2:23 pm on April 1, 2026: member

    that would require changes to mp::ListenConnections in libmultiprocess

    Good question. libmultiprocess should probably give us a hook to (optionally) control the maximum number of connections. It might be good to draft a PR for that on bitcoin-core/libmultiprocess, or at least open an issue.

    It doesn’t have to a pre-requisite for this PR, it could indeed be a followup.

  9. enirox001 force-pushed on Apr 1, 2026
  10. in src/init.cpp:167 in b1abf1a5e1
    163@@ -164,6 +164,7 @@ static constexpr bool DEFAULT_STOPAFTERBLOCKIMPORT{false};
    164 #endif
    165 
    166 static constexpr int MIN_CORE_FDS = MIN_LEVELDB_FDS + NUM_FDS_MESSAGE_CAPTURE;
    167+static constexpr int DEFAULT_IPC_MAX_CONNECTIONS{16};
    


    kevkevinpal commented at 3:04 pm on April 1, 2026:
    I would update this to reflect the arg name DEFAULT_UNIX_MAX_CONNECTIONS

    enirox001 commented at 3:19 pm on April 1, 2026:
    Done, thanks
  11. in src/init.cpp:1048 in b1abf1a5e1
    1043+
    1044+    // We only need to reserve these FDs if the user is actually binding an IPC socket
    1045+    int user_ipc_max_connections = 0;
    1046+
    1047+    if (args.IsArgSet("-ipcbind")) {
    1048+        user_ipc_max_connections = args.GetIntArg("-unixmaxconnections", DEFAULT_IPC_MAX_CONNECTIONS);
    


    kevkevinpal commented at 3:05 pm on April 1, 2026:
    same here to update this to reflect the arg name user_unix_max_connections

    enirox001 commented at 3:20 pm on April 1, 2026:
    Done, made this update
  12. kevkevinpal commented at 3:07 pm on April 1, 2026: contributor

    Concept ACK b1abf1a

    I would also update the description to reflect the usage of -unixmaxconnections

  13. enirox001 force-pushed on Apr 1, 2026
  14. enirox001 commented at 3:27 pm on April 1, 2026: contributor

    Thanks for the reviews @Sjors @kevkevinpal.

    I have addressed the comments in the recent push, also updated the PR description to match the changes made.

  15. w0xlt commented at 11:45 pm on April 1, 2026: contributor
    Concept ACK
  16. enirox001 force-pushed on Apr 2, 2026
  17. enirox001 force-pushed on Apr 3, 2026
  18. enirox001 commented at 12:20 pm on April 3, 2026: contributor
    C.I failure is unrelated
  19. in src/init.cpp:1050 in 37e195d99e outdated
    1045+    int user_unix_max_connections = 0;
    1046+
    1047+    if (args.IsArgSet("-ipcbind")) {
    1048+        user_unix_max_connections = args.GetIntArg("-unixmaxconnections", DEFAULT_UNIX_MAX_CONNECTIONS);
    1049+        if (user_unix_max_connections < 0) {
    1050+            return InitError(Untranslated("-unixmaxconnections must be greater than or equal to zero"));
    


    kevkevinpal commented at 12:35 pm on April 3, 2026:
    Can we test this in the functional test?

    enirox001 commented at 12:57 pm on April 3, 2026:
    Good suggeestion, added this in the recent push. Thanks
  20. in src/init.cpp:1047 in 1c0a5e38de outdated
    1042     }
    1043+
    1044+    // We only need to reserve these FDs if the user is actually binding an IPC socket
    1045+    int user_unix_max_connections = 0;
    1046+
    1047+    if (args.IsArgSet("-ipcbind")) {
    


    ryanofsky commented at 12:48 pm on April 3, 2026:

    In commit “init: Reserve file descriptors for IPC connections” (fa57007dfde3a532f76709b8ff0f38e52f7412aa)

    This line should be changed to if (ipc_bind > 0) so it will work correctly when -noipcbind is specified. (In general IsArgSet should be avoided because it works poorly with negation).

    The other check on line 1055 will have similar problems. Would suggest something more like:

     0auto ipc_bind = args.GetArgs("-ipcbind").size();
     1auto unix_max_connections = args.GetIntArg("-unixmaxconnections", 0);
     2
     3if (unix_max_connections < 0) {
     4    return InitError(Untranslated("-unixmaxconnections must be greater than or equal to zero"));
     5} else if (ipc_bind > 0) {
     6    if (!IsArgSet("-unixmaxconnections")) unix_max_connections = DEFAULT_UNIX_MAX_CONNECTIONS;
     7    LogInfo("Reserving %d file descriptors for IPC (%d listening sockets, %d accepted connections)",
     8                    ipc_bind + user_unix_max_connections , ipc_bind, user_unix_max_connections);
     9} else if (unix_max_connections > 0) {
    10    LogWarning("-unixmaxconnections is %s but -ipcbind is not enabled; option will have no effect.\n", unix_max_connections);
    11    unix_max_connections = 0;
    12}
    

    enirox001 commented at 5:30 pm on April 3, 2026:

    Good catch.

    Updated to check ipc_bind > 0 instead of IsArgSet("-ipcbind") so it handles -noipcbind correctly, and restructured the logic as suggested.

  21. enirox001 force-pushed on Apr 3, 2026
  22. in src/init.cpp:724 in 1c0a5e38de
    720@@ -720,6 +721,7 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc)
    721     argsman.AddArg("-server", "Accept command line and JSON-RPC commands", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
    722     if (can_listen_ipc) {
    723         argsman.AddArg("-ipcbind=<address>", "Bind to Unix socket address and listen for incoming connections. Valid address values are \"unix\" to listen on the default path, <datadir>/node.sock, or \"unix:/custom/path\" to specify a custom path. Can be specified multiple times to listen on multiple paths. Default behavior is not to listen on any path. If relative paths are specified, they are interpreted relative to the network data directory. If paths include any parent directory components and the parent directories do not exist, they will be created. Enabling this gives local processes that can access the socket unauthenticated RPC access, so it's important to choose a path with secure permissions if customizing this.", ArgsManager::ALLOW_ANY, OptionsCategory::IPC);
    724+        argsman.AddArg("-unixmaxconnections=<n>", "Reserve file descriptors for up to <n> concurrent IPC socket connections (default: " + ToString(DEFAULT_UNIX_MAX_CONNECTIONS) + ")", ArgsManager::ALLOW_ANY, OptionsCategory::IPC);
    


    ryanofsky commented at 1:17 pm on April 3, 2026:

    In commit “init: Reserve file descriptors for IPC connections” (fa57007dfde3a532f76709b8ff0f38e52f7412aa)

    I think it would be better to call this -ipcmaxconnections to more clearly connect this to other IPC parameters like -ipcbind and -ipcconnect.


    enirox001 commented at 5:48 pm on April 3, 2026:
    Renamed to -ipcmaxconnections to keep it consistent with -ipcbind and -ipcconnect.
  23. ryanofsky approved
  24. ryanofsky commented at 1:32 pm on April 3, 2026: contributor

    Code review ac1e51db8639e194967454ee0a4da0e8dd7949a0. This looks pretty good, but it looks like there is a problem with negation (see below), and also I made a suggestion about renaming the new option.

    Ideally, I think I would want to implement the max connection limit by extending the -ipcbind option instead of introducing a new option. I’d probably extend the -ipcbind syntax to support socat-style options like -ipcbind=unix:/path/to/socket:max-connections=8 when listening on a custom path or -ipcbind=unix::max-connections=10 when listening on the default path. This way different paths (or different transports if we add tcp support) could have different connection limits.

    But I can see that a separate option is probably easiest to implement, and it might be useful to users to have an overall cap on the number of connections, so current approach makes sense and might be best for now.

  25. DrahtBot removed the label CI failed on Apr 3, 2026
  26. enirox001 force-pushed on Apr 3, 2026
  27. enirox001 commented at 5:40 pm on April 3, 2026: contributor

    Thanks for the reviews so far @ryanofsky @kevkevinpal @Sjors

    Ideally, I think I would want to implement the max connection limit by extending the -ipcbind option instead of introducing a new option. I’d probably extend the -ipcbind syntax to support socat-style options like -ipcbind=unix:/path/to/socket:max-connections=8 when listening on a custom path or -ipcbind=unix::max-connections=10 when listening on the default path. This way different paths (or different transports if we add tcp support) could have different connection limits.

    The socat-style syntax for per-path limits is an interesting idea. Happy to follow up with that approach in a separate PR if there’s appetite for it

    Changes made in this commit are:

    • Renamed to -ipcmaxconnections to keep it consistent with -ipcbind and -ipcconnect. @Sjors had suggested, unixmaxconnections to be more transport-specific, but -ipcmaxconnections seems clearer given the current codebase only has Unix socket IPC.
    • Fixed the IsArgSet negation bug by restructuring the logic to check ipc_bind > 0 instead, so -noipcbind is handled correctly:
    • Also extended the functional test to assert that passing -ipcmaxconnections=-1 raises the expected init error.
  28. in test/functional/interface_ipc_cli.py:38 in dc0583bb8b
    30@@ -31,16 +31,27 @@ def test_cli(self, args, error=None):
    31         if error is None:
    32             assert_equal(result.stdout, '[\n  "foo"\n]\n')
    33         else:
    34-            assert result.stdout.startswith(error), f"Output didn't start with the expected error {error!r}:\n{result.stdout}"
    35+            assert_equal(result.stdout, error)
    36         assert_equal(result.stderr, None)
    37         assert_equal(result.returncode, 0 if error is None else 1)
    38 
    39+    def test_ipcmaxconnections(self):
    


    ryanofsky commented at 3:14 pm on April 6, 2026:

    In commit “test: assert file descriptors available is logged on -ipcbind startup” (dc0583bb8b78ba11c824dc5cb628ab7835f88aee)

    I think this file is probably not the right place for these checks, because this file is trying to test bitcoin-cil. Would suggest moving this to interface_ipc.py or adding a new test like interface_ipc_init.py


    enirox001 commented at 8:39 am on April 8, 2026:
    Good point, this didn’t really belong in interface_ipc_cli.py. I moved the startup logging coverage into a dedicated interface_ipc_init.py test so the tests are more focused on their intended use
  29. in test/functional/interface_ipc_cli.py:43 in bf037b7476
    36@@ -37,10 +37,27 @@ def test_cli(self, args, error=None):
    37 
    38     def test_ipcmaxconnections(self):
    39         self.log.info("Test -ipcmaxconnections")
    40+        node = self.nodes[0]
    41 
    42-        # Verify FD availability is logged at startup with -ipcbind.
    43-        with self.nodes[0].assert_debug_log(["file descriptors available"]):
    44-            self.restart_node(0)
    


    ryanofsky commented at 3:18 pm on April 6, 2026:

    In commit “test: extend -ipcmaxconnections coverage to assert FD reservation logging” (bf037b74762220afab8d506c56f7d801b5e6da7e)

    Is there a reason for deleting this test? Maybe it should just be extended to check for the new log string?


    enirox001 commented at 8:41 am on April 8, 2026:
    Agreed, I kept the original “file descriptors available” check and extended it in interface_ipc_init.py so the same startup also asserts the new IPC FD reservation log line.
  30. in test/functional/interface_ipc_cli.py:60 in bf037b7476
    58+        )
    59+        self.start_node(0)
    60+
    61+        # Verify FD reservation is explicitly logged with correct values
    62+        with node.assert_debug_log(["Reserving 9 file descriptors for IPC (1 listening sockets, 8 accepted connections)"]):
    63+            self.restart_node(0, extra_args=["-ipcmaxconnections=8"])
    


    ryanofsky commented at 3:22 pm on April 6, 2026:

    In commit “test: extend -ipcmaxconnections coverage to assert FD reservation logging” (bf037b74762220afab8d506c56f7d801b5e6da7e)

    It would seem good to just make this the first test and only start the node with -ipcmaxconnections=8 one time, instead of twice.


    enirox001 commented at 8:41 am on April 8, 2026:
    Changed that so the first startup uses -ipcmaxconnections=8, and that single restart now checks both the existing startup log and the new reservation log.
  31. ryanofsky approved
  32. ryanofsky commented at 4:29 pm on April 6, 2026: contributor

    Code review ACK bf037b74762220afab8d506c56f7d801b5e6da7e. Strictly speaking I think this PR is an improvement over the status quo, because right now no file descriptors are reserved for IPC and this does reserve some.

    But I have the same concerns as Sjors #34978#pullrequestreview-4042683109 about how usable the new command line option will be because it only reserves file descriptors for IPC, without limiting how many descriptors IPC uses.

    I was also initially concerned that a global limit on number of incoming connections could be more awkward to implement than local limits per listening address, because implementing local limits would just seem to require adding a max_connections parameter to mp::ListenConnections and using a local counter to stop calling ConnectionReceiver::accept when the limit is reached, while a global limit would require adding more state to the EventLoop class. But probably overall complexity of both approaches is about the same.

    I’d still prefer local limits, and accepting socat-style max-connections=<n> options in -ipcbind over introducing a new command line option and global limit, but that’s not a strong preference.

    It would also be nice to see a draft PR with a some complete implementation of connection limiting because it would be a natural followup and could help compare the different approaches.

  33. DrahtBot requested review from Sjors on Apr 6, 2026
  34. DrahtBot requested review from kevkevinpal on Apr 6, 2026
  35. enirox001 force-pushed on Apr 8, 2026
  36. test: assert file descriptors available is logged on -ipcbind startup
    Before adding IPC FD reservation, verify that the existing "file
    descriptors available" log line fires when the node starts with
    -ipcbind=unix. This establishes a baseline; the following commits
    rename this helper to test_ipcmaxconnections and extend it to assert
    the new reservation log line.
    f7e40ee984
  37. init: Reserve file descriptors for IPC connections
    When -ipcbind is used, the node opens one listening socket FD per
    bound address and accepts concurrent IPC connections. Neither was
    previously accounted for in min_required_fds, meaning IPC-heavy
    workloads could silently exhaust available file descriptors at
    unpredictable moments.
    
    Add -ipcmaxconnections (default: 16, mirroring -rpcworkqueue) so
    operators can control how many FDs are reserved for accepted IPC
    connections. Add ipc_bind to account for the listening socket FDs
    opened per -ipcbind address, which were previously unaccounted for.
    Emit a warning when -ipcmaxconnections is set without -ipcbind. Log
    the total IPC FD reservation at startup so operators can verify it.
    
    Suggested by Sjors in #32297 (comment).
    9139dfd704
  38. test: extend -ipcmaxconnections coverage to assert FD reservation logging
    Add -ipcmaxconnections startup checks and assert the new IPC FD
    reservation log line fires with correct values. With -ipcmaxconnections=8,
    the node logs "Reserving 9 file descriptors for IPC (1 listening sockets,
    8 accepted connections)", demonstrating that this PR increases the reserved
    FD count.
    365be355a5
  39. enirox001 force-pushed on Apr 8, 2026
  40. DrahtBot added the label CI failed on Apr 8, 2026
  41. DrahtBot removed the label CI failed on Apr 8, 2026
  42. enirox001 commented at 10:05 am on April 8, 2026: contributor

    Thanks everyone for the review so far.

    I addressed the test-related comments by moving the startup logging coverage out of interface_ipc_cli.py into a dedicated interface_ipc_init.py test. The original “file descriptors available” check is still there and is now extended to also assert the new IPC FD reservation log line, with the -ipcmaxconnections=8 case using a single restart for both checks.

    I also opened an upstream draft PR to prototype local per-listener connection limiting, which would be the natural follow-up for enforcing a real IPC connection cap instead of only reserving FDs.

  43. ryanofsky approved
  44. ryanofsky commented at 2:55 pm on April 8, 2026: contributor

    Code review ACK bf037b74762220afab8d506c56f7d801b5e6da7e. Thanks for the test updates!

    One caveat: after reviewing https://github.com/bitcoin-core/libmultiprocess/pull/269 I’m less sure that we will want to add a global -ipcmaxconnections option if we can add per-address limits instead (with -ipcbind max-connections options or something similar).

    Either approach should work but should probably pick one before merging any of these PRs.

  45. enirox001 commented at 1:51 pm on April 9, 2026: contributor

    I opened a separate draft PR here #35037 to make the per-address approach concrete downstream using the local listener limit API from bitcoin-core/libmultiprocess#269.

    That draft uses -ipcbind ... max-connections=<n> instead of a global -ipcmaxconnections option so the two approaches can be compared more directly.

  46. ViniciusCestarii commented at 3:01 pm on April 9, 2026: contributor

    There doesn’t seem to be a test covering the default behavior when -ipcmaxconnections is not set.

    It might be useful to add a case asserting the expected default (16 connections) at test/functional/interface_ipc_init.py:

    0        with node.assert_debug_log([
    1            "file descriptors available",
    2            "Reserving 17 file descriptors for IPC (1 listening sockets, 16 accepted connections)",
    3        ]):
    4            self.restart_node(0)
    5        assert_equal(node.getblockcount(), 0)
    

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-12 09:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me