Skip to content

Add std::unordered_set support and a helper BuildList to dedup list build handlers#277

Merged
ryanofsky merged 3 commits into
bitcoin-core:masterfrom
ViniciusCestarii:add-unordered-set-support
Jun 9, 2026
Merged

Add std::unordered_set support and a helper BuildList to dedup list build handlers#277
ryanofsky merged 3 commits into
bitcoin-core:masterfrom
ViniciusCestarii:add-unordered-set-support

Conversation

@ViniciusCestarii

@ViniciusCestarii ViniciusCestarii commented May 14, 2026

Copy link
Copy Markdown
Contributor

Add std::unordered_set support and a helper BuildList to dedup list build handlers that is being used for map, set and vector.

While looking bitcoin/bitcoin#29409, found a TODO noting that libmultiprocess lacked std::unordered_set support, requiring downstream that PR to implement the build/read functions locally.

I believe there could be more dedup adding a ReadList too. I could do that as a follow-up if desirable

@DrahtBot

DrahtBot commented May 14, 2026

Copy link
Copy Markdown

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

  • #288 (Create support branch for CI scripts, documentation, and examples by ryanofsky)
  • #276 (build: prepare for subtree split by Sjors)

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.

@ryanofsky ryanofsky left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review ACK 00c9a8e. This is a well-implemented change that simplifies bitcoin/bitcoin#29409, and could help other places, since unordered_sets are used for various things in bitcoin core, and it makes sense to allow them to be easily passed over capnproto interfaces.

I believe there could be more dedup adding a ReadList too. I could do that as a follow-up if desirable

Yes this would be a nice followup, especially since type-set.h and type-unordered-set.h files are almost exact duplicates of each other and other read functions are very similar. Might be possible to write a generic ReadList that works for all containers by taking an init(size) callback and an emplace(args...) callback.

@ViniciusCestarii

Copy link
Copy Markdown
Contributor Author

The new optional_int field on master uses a proper camelCase capnp name (optionalInt) with $Proxy.name mapping to a snake_case C++ member, which reads cleaner than the older lowercase style (setint, vbool). Should I follow that pattern here and rename unorderedsetint to an unorderedSetInt capnp field with a snake_case C++ member?

No need to touch the existing setint/vbool fields here. I believe that cleanup could be a separate PR.

@ryanofsky

Copy link
Copy Markdown
Collaborator

Should I follow that pattern here and rename unorderedsetint to an unorderedSetInt capnp field with a snake_case C++ member?

Anything seems fine to me, would choose whatever seems cleanest to you. Feel free to include a commit renaming existing fields in this PR too if you'd like

@ViniciusCestarii ViniciusCestarii force-pushed the add-unordered-set-support branch from 00c9a8e to 81e508f Compare June 3, 2026 17:04
@ViniciusCestarii ViniciusCestarii force-pushed the add-unordered-set-support branch from 81e508f to 9de4b88 Compare June 3, 2026 17:06
@ViniciusCestarii

Copy link
Copy Markdown
Contributor Author

Forced-push 9de4b88 rebasing and fixing conflict with master and added the commit 9de4b88 "test: use camelCase + $Proxy.name for FooStruct fields"

@ryanofsky ryanofsky left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review ACK 9de4b88. Only changes since last review were renaming and adding commit user better field names in the test.

Comment thread test/mp/test/foo.h
std::set<int> setint;
std::vector<bool> vbool;
std::set<int> set_int;
std::vector<bool> v_bool;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In commit "test: use camelCase + $Proxy.name for FooStruct fields" (9de4b88)

Not important but vector_bool would seem more consistent with other field names

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. If in the future I touch these code lines again I will apply this suggestion. Thanks for the feedback!

@ryanofsky ryanofsky merged commit e7e91b2 into bitcoin-core:master Jun 9, 2026
12 of 13 checks passed
@ViniciusCestarii ViniciusCestarii deleted the add-unordered-set-support branch June 10, 2026 12:18
Sjors added a commit to Sjors/sv2-tp that referenced this pull request Jun 17, 2026
8412fcdc65 Merge bitcoin-core/libmultiprocess#295: Mark Waiter m_cv as guarded by m_mutex
1593ee2d18 Merge bitcoin-core/libmultiprocess#294: test: Add passDouble smoke test
9885d7dd33 Merge bitcoin-core/libmultiprocess#286: proxy-client: fix TSan data race in clientDestroy
fa35501c4f Mark Waiter m_cv as guarded by m_mutex
faaedb11f8 test: Add passDouble smoke test
733c64318d Merge bitcoin-core/libmultiprocess#292: type-number: fix clang-tidy modernize-use-nullptr
9cc3479ab3 Merge bitcoin-core/libmultiprocess#291: cmake: Add `mp_headers` custom target
201abd9e3a Merge bitcoin-core/libmultiprocess#289: cmake: make target_capnp_sources use CURRENT dirs
99820c8aec Merge bitcoin-core/libmultiprocess#279: doc: Add comments to FIELD_* constants in proxy.h
73b985540c Merge bitcoin-core/libmultiprocess#278: doc: Fix and expand design.md
e7e91b2e23 Merge bitcoin-core/libmultiprocess#277: Add std::unordered_set support and a helper BuildList to dedup list build handlers
91a951f59a tidy fix: modernize-use-nullptr
16362f42d0 cmake: Add `mp_headers` custom target
615a94fe3a cmake: document ONLY_CAPNP option in target_capnp_sources
90982f75c6 mpgen: iwyu changes required by previous commit
25bb3e67f3 proxy-client: fix TSan data race in clientDestroy
620f297f31 cmake: make target_capnp_sources use CURRENT dirs
9de4b885aa test: use camelCase + $Proxy.name for FooStruct fields
011b91793d type: add std::unordered_set support
20d19b9644 proxy: add BuildList helper and dedup map/set/vector build handlers
e863c6cdf6 doc: Add comments to FIELD_* constants in proxy.h
18db0ab957 doc: Fix and expand design.md
61de697536 Merge bitcoin-core/libmultiprocess#273: proxy-client: tolerate exceptions from remote destroy during cleanup
9cec9d6ca5 Merge bitcoin-core/libmultiprocess#243: mpgen: support primitive std::optional struct fields
4aaff11374 Merge bitcoin-core/libmultiprocess#238: cmake, ci: updates for recent nixpkgs
2ac55a56b5 Merge bitcoin-core/libmultiprocess#218: Better error and log messages
6de92e1c73 proxy-client: tolerate exceptions from remote destroy during cleanup
90be8354d4 test: regression for ~ProxyClient destroy after peer disconnect
3c69d125a1 Merge bitcoin-core/libmultiprocess#260: event loop: tolerate unexpected exceptions in `post()` callbacks
b8a48c65e6 event loop: tolerate unexpected exceptions in `post()` callbacks
f787863d2c Merge bitcoin-core/libmultiprocess#270: doc: Bump version 10 > 11
a22f602910 doc: Bump version 10 > 11
4eae445d6d debug: Add TypeName() function and log statements for Proxy objects being created and destroyed
f326c5b1b7 logging: Add better logging on IPC server-side failures
6dbfa56a04 mpgen: support primitive std::optional struct fields
8d1277deb5 mpgen refactor: add AccessorType function
db716bbcba mpgen refactor: Move field handling code to FieldList class
db7acb3ce2 ci: Fix shell.nix compatibility with CMake 4.0
91a7759a9a cmake: Fix IWYU in nix by adding CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES

git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: 8412fcdc659e1379f9b4dea896c26bc1c5f3afa8
enirox001 added a commit to enirox001/bitcoin that referenced this pull request Jun 29, 2026
2cfc3b4ad4 mpgen: add missing includes for IWYU compliance
bd1d2fc33a proxy: add local connection limit to ListenConnections
40b2e97b27 test: add dedicated ListenConnections coverage
e5eba302cd doc/version: Bump version 11 > 12
8412fcdc65 Merge bitcoin-core/libmultiprocess#295: Mark Waiter m_cv as guarded by m_mutex
1593ee2d18 Merge bitcoin-core/libmultiprocess#294: test: Add passDouble smoke test
9885d7dd33 Merge bitcoin-core/libmultiprocess#286: proxy-client: fix TSan data race in clientDestroy
fa35501c4f Mark Waiter m_cv as guarded by m_mutex
faaedb11f8 test: Add passDouble smoke test
733c64318d Merge bitcoin-core/libmultiprocess#292: type-number: fix clang-tidy modernize-use-nullptr
9cc3479ab3 Merge bitcoin-core/libmultiprocess#291: cmake: Add `mp_headers` custom target
201abd9e3a Merge bitcoin-core/libmultiprocess#289: cmake: make target_capnp_sources use CURRENT dirs
99820c8aec Merge bitcoin-core/libmultiprocess#279: doc: Add comments to FIELD_* constants in proxy.h
73b985540c Merge bitcoin-core/libmultiprocess#278: doc: Fix and expand design.md
e7e91b2e23 Merge bitcoin-core/libmultiprocess#277: Add std::unordered_set support and a helper BuildList to dedup list build handlers
91a951f59a tidy fix: modernize-use-nullptr
16362f42d0 cmake: Add `mp_headers` custom target
615a94fe3a cmake: document ONLY_CAPNP option in target_capnp_sources
90982f75c6 mpgen: iwyu changes required by previous commit
25bb3e67f3 proxy-client: fix TSan data race in clientDestroy
620f297f31 cmake: make target_capnp_sources use CURRENT dirs
9de4b885aa test: use camelCase + $Proxy.name for FooStruct fields
011b91793d type: add std::unordered_set support
20d19b9644 proxy: add BuildList helper and dedup map/set/vector build handlers
e863c6cdf6 doc: Add comments to FIELD_* constants in proxy.h
18db0ab957 doc: Fix and expand design.md
61de697536 Merge bitcoin-core/libmultiprocess#273: proxy-client: tolerate exceptions from remote destroy during cleanup
9cec9d6ca5 Merge bitcoin-core/libmultiprocess#243: mpgen: support primitive std::optional struct fields
4aaff11374 Merge bitcoin-core/libmultiprocess#238: cmake, ci: updates for recent nixpkgs
2ac55a56b5 Merge bitcoin-core/libmultiprocess#218: Better error and log messages
6de92e1c73 proxy-client: tolerate exceptions from remote destroy during cleanup
90be8354d4 test: regression for ~ProxyClient destroy after peer disconnect
3c69d125a1 Merge bitcoin-core/libmultiprocess#260: event loop: tolerate unexpected exceptions in `post()` callbacks
b8a48c65e6 event loop: tolerate unexpected exceptions in `post()` callbacks
f787863d2c Merge bitcoin-core/libmultiprocess#270: doc: Bump version 10 > 11
a22f602910 doc: Bump version 10 > 11
4eae445d6d debug: Add TypeName() function and log statements for Proxy objects being created and destroyed
f326c5b1b7 logging: Add better logging on IPC server-side failures
6dbfa56a04 mpgen: support primitive std::optional struct fields
8d1277deb5 mpgen refactor: add AccessorType function
db716bbcba mpgen refactor: Move field handling code to FieldList class
db7acb3ce2 ci: Fix shell.nix compatibility with CMake 4.0
91a7759a9a cmake: Fix IWYU in nix by adding CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES

git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: 2cfc3b4ad41695e810ac7b6e43e00f59a393d0cb
ryanofsky added a commit that referenced this pull request Jul 5, 2026
6450345 type: reserve first when reading std::unordered_set (ViniciusCestarii)
4d0f8db proxy: add ReadList helper and dedup map/set/vector read handlers (ViniciusCestarii)

Pull request description:

  Follow up of #277 (review). This adds `ReadList` helper  and uses it to dedup the `CustomReadField` implementations for `std::map`, `std::set`, `std::unordered_set`, and `std::vector`, mirroring the existing `BuildList` helper on the build side.

  I took the liberty of adding the commit f2a734a "type: reserve first when reading std::unordered_set" so it reserves the correct capacity first and then emplaces the values.

ACKs for top commit:
  ryanofsky:
    Code review ACK 6450345. Looks great! It's really nice to deduplicate this code and drop `vector<bool>` specialization.

Tree-SHA512: f8d44caccb5265e4d2206556514e099ad37ff958ae35ff08eea0190a1850178e2a9abfb5bc083491412f4439570f95d82e8a665fd4f452084dfd11a5167b4547
fanquake pushed a commit to bitcoin/bitcoin that referenced this pull request Jul 7, 2026
…ea02

16bf05dea02 Merge bitcoin-core/libmultiprocess#302: refactor: rename EventLoop::m_num_clients to m_num_refs
dd537da9e40 Merge bitcoin-core/libmultiprocess#301: test: recursive async IPC calls and cleanups
400291de000 Merge bitcoin-core/libmultiprocess#299: ci: remove libevent from Core CIs
092be515adf Merge bitcoin-core/libmultiprocess#285: Add ReadList helper
5b617880c51 Merge bitcoin-core/libmultiprocess#283: Add `makePool` method on `ThreadMap`
d4998304154 refactor: rename EventLoop::m_num_clients to m_num_refs
6450345c985 type: reserve first when reading std::unordered_set
4d0f8db5f99 proxy: add ReadList helper and dedup map/set/vector read handlers
0e49d911867 Add `makePool` method on `ThreadMap`
5519f7f9485 test: recursive async IPC calls
a29ceff40bc ci: remove libevent from Core CIs
8412fcdc659 Merge bitcoin-core/libmultiprocess#295: Mark Waiter m_cv as guarded by m_mutex
1593ee2d18a Merge bitcoin-core/libmultiprocess#294: test: Add passDouble smoke test
9885d7dd33c Merge bitcoin-core/libmultiprocess#286: proxy-client: fix TSan data race in clientDestroy
fa35501c4f0 Mark Waiter m_cv as guarded by m_mutex
faaedb11f8a test: Add passDouble smoke test
733c64318d1 Merge bitcoin-core/libmultiprocess#292: type-number: fix clang-tidy modernize-use-nullptr
9cc3479ab33 Merge bitcoin-core/libmultiprocess#291: cmake: Add `mp_headers` custom target
201abd9e3a5 Merge bitcoin-core/libmultiprocess#289: cmake: make target_capnp_sources use CURRENT dirs
99820c8aecb Merge bitcoin-core/libmultiprocess#279: doc: Add comments to FIELD_* constants in proxy.h
73b985540c5 Merge bitcoin-core/libmultiprocess#278: doc: Fix and expand design.md
e7e91b2e23e Merge bitcoin-core/libmultiprocess#277: Add std::unordered_set support and a helper BuildList to dedup list build handlers
91a951f59ac tidy fix: modernize-use-nullptr
16362f42d01 cmake: Add `mp_headers` custom target
615a94fe3a2 cmake: document ONLY_CAPNP option in target_capnp_sources
90982f75c6b mpgen: iwyu changes required by previous commit
25bb3e67f39 proxy-client: fix TSan data race in clientDestroy
620f297f311 cmake: make target_capnp_sources use CURRENT dirs
9de4b885aa6 test: use camelCase + $Proxy.name for FooStruct fields
011b91793dd type: add std::unordered_set support
20d19b9644e proxy: add BuildList helper and dedup map/set/vector build handlers
e863c6cdf61 doc: Add comments to FIELD_* constants in proxy.h
18db0ab9570 doc: Fix and expand design.md
61de6975362 Merge bitcoin-core/libmultiprocess#273: proxy-client: tolerate exceptions from remote destroy during cleanup
9cec9d6ca55 Merge bitcoin-core/libmultiprocess#243: mpgen: support primitive std::optional struct fields
4aaff113745 Merge bitcoin-core/libmultiprocess#238: cmake, ci: updates for recent nixpkgs
2ac55a56b58 Merge bitcoin-core/libmultiprocess#218: Better error and log messages
6de92e1c732 proxy-client: tolerate exceptions from remote destroy during cleanup
90be8354d47 test: regression for ~ProxyClient destroy after peer disconnect
3c69d125a17 Merge bitcoin-core/libmultiprocess#260: event loop: tolerate unexpected exceptions in `post()` callbacks
b8a48c65e60 event loop: tolerate unexpected exceptions in `post()` callbacks
f787863d2cd Merge bitcoin-core/libmultiprocess#270: doc: Bump version 10 > 11
a22f6029103 doc: Bump version 10 > 11
4eae445d6d8 debug: Add TypeName() function and log statements for Proxy objects being created and destroyed
f326c5b1b7b logging: Add better logging on IPC server-side failures
6dbfa56a040 mpgen: support primitive std::optional struct fields
8d1277deb55 mpgen refactor: add AccessorType function
db716bbcba7 mpgen refactor: Move field handling code to FieldList class
db7acb3ce27 ci: Fix shell.nix compatibility with CMake 4.0
91a7759a9ab cmake: Fix IWYU in nix by adding CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES

git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: 16bf05dea02651f75733ff08531181aa774fc5a8
fanquake added a commit to bitcoin/bitcoin that referenced this pull request Jul 7, 2026
…ol` method

6b0a907 Squashed 'src/ipc/libmultiprocess/' changes from 3edbe8f67c1..16bf05dea02 (Ryan Ofsky)

Pull request description:

  The changes can be verified by running `test/lint/git-subtree-check.sh src/ipc/libmultiprocess` as described in [developer notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#subtrees) and [lint instructions](https://github.com/bitcoin/bitcoin/tree/master/test/lint#git-subtree-checksh).

  Changes since last subtree update (#34977):

  - Adds `makePool` method on `ThreadMap` to support thread pool routing, allowing requests without a specific client thread to be dispatched to a pool using a shortest-queue strategy ([#283](bitcoin-core/libmultiprocess#283)).
  - Adds `std::unordered_set` support, a `BuildList` helper, and a `ReadList` helper to reduce duplication in list build and read handlers ([#277](bitcoin-core/libmultiprocess#277), [#285](bitcoin-core/libmultiprocess#285)).
  - Adds support for translating C++ `std::optional<T>` struct fields to pairs of `T` + `hasT :Bool` Cap'n Proto struct fields, allowing unset optional primitive fields to be represented ([#243](bitcoin-core/libmultiprocess#243)).
  - Produces more readable log output for Proxy object lifecycle events and IPC server-side failures ([#218](bitcoin-core/libmultiprocess#218)).
  - Handles exceptions thrown by `destroy` methods by logging instead of aborting ([#273](bitcoin-core/libmultiprocess#273)). This can prevent server crashes when non-libmultiprocess clients disconnect without destroying objects, in the case where a server object owns client objects and the server destructor tries to call the disconnected client to free them ([#219](bitcoin-core/libmultiprocess#219)).
  - Handles unexpected exceptions thrown by callbacks (that should never happen) by logging errors instead of deadlocking ([#260](bitcoin-core/libmultiprocess#260)).
  - Fixes a rare mptest hang on musl builds caused by a lost wakeup bug in `Waiter` ([#295](bitcoin-core/libmultiprocess#295)).
  - Fixes a race condition in a log print detected by TSan ([#286](bitcoin-core/libmultiprocess#286)).
  - Build improvements: makes `target_capnp_sources` work correctly when libmultiprocess is used as a CMake subproject ([#289](bitcoin-core/libmultiprocess#289)), adds `mp_headers` target for better lint tool support ([#291](bitcoin-core/libmultiprocess#291)), and fixes compatibility with recent Nix and CMake 4.0 ([#238](bitcoin-core/libmultiprocess#238)).
  - Test, CI, documentation, and minor code improvements: design document corrections ([#278](bitcoin-core/libmultiprocess#278)), field constant comments ([#279](bitcoin-core/libmultiprocess#279)), clang-tidy fix ([#292](bitcoin-core/libmultiprocess#292)), new smoke test for double-precision float values ([#294](bitcoin-core/libmultiprocess#294)), new test for recursive async IPC calls ([#301](bitcoin-core/libmultiprocess#301)), removal of libevent from Core CI builds ([#299](bitcoin-core/libmultiprocess#299)), and rename of `EventLoop::m_num_clients` to `m_num_refs` ([#302](bitcoin-core/libmultiprocess#302)).

ACKs for top commit:
  fanquake:
    ACK 02afa66
  hebasto:
    ACK 02afa66.

Tree-SHA512: ef81a951c971f328a0a98436030467eeea30925eb6016eafd9bc7a25726c87628a852bbb1d84b88bce340aeea2bed25c65bc55db1168ebcb850628cd18808883
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants