init: Reserve file descriptors for IPC connections #34978

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

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 <code></code> 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.

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.

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.

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.

DrahtBot added the label CI failed on Apr 1, 2026

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?

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.

enirox001 force-pushed on Apr 1, 2026

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

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

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

enirox001 force-pushed on Apr 1, 2026

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.

w0xlt commented at 11:45 PM on April 1, 2026: contributor

Concept ACK

enirox001 force-pushed on Apr 2, 2026

enirox001 force-pushed on Apr 3, 2026

enirox001 commented at 12:20 PM on April 3, 2026: contributor

C.I failure is unrelated

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

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:

auto ipc_bind = args.GetArgs("-ipcbind").size();
auto unix_max_connections = args.GetIntArg("-unixmaxconnections", 0);

if (unix_max_connections < 0) {
    return InitError(Untranslated("-unixmaxconnections must be greater than or equal to zero"));
} else if (ipc_bind > 0) {
    if (!IsArgSet("-unixmaxconnections")) unix_max_connections = DEFAULT_UNIX_MAX_CONNECTIONS;
    LogInfo("Reserving %d file descriptors for IPC (%d listening sockets, %d accepted connections)",
                    ipc_bind + user_unix_max_connections , ipc_bind, user_unix_max_connections);
} else if (unix_max_connections > 0) {
    LogWarning("-unixmaxconnections is %s but -ipcbind is not enabled; option will have no effect.\n", unix_max_connections);
    unix_max_connections = 0;
}

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.

enirox001 force-pushed on Apr 3, 2026

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.

ryanofsky approved

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.

DrahtBot removed the label CI failed on Apr 3, 2026

enirox001 force-pushed on Apr 3, 2026

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.

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

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.

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.

ryanofsky approved

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.

DrahtBot requested review from Sjors on Apr 6, 2026

DrahtBot requested review from kevkevinpal on Apr 6, 2026

enirox001 force-pushed on Apr 8, 2026

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

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

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

enirox001 force-pushed on Apr 8, 2026

DrahtBot added the label CI failed on Apr 8, 2026

DrahtBot removed the label CI failed on Apr 8, 2026

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.

ryanofsky approved

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.

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.

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:

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