Commit 3b0323a
committed
Merge #6007: refactor: move {
bd8b5d4 net: add more details to log information in ETE and `WakeupPipes` (Kittywhiskers Van Gogh)
ec99294 net: restrict access `EdgeTriggerEvents` members (Kittywhiskers Van Gogh)
f24520a net: log `close` failures in `EdgeTriggerEvents` and `WakeupPipe` (Kittywhiskers Van Gogh)
b8c3b48 refactor: introduce `WakeupPipe`, move wakeup select pipe logic there (Kittywhiskers Van Gogh)
ed7d976 refactor: move wakeup pipe (de)registration to ETE (Kittywhiskers Van Gogh)
f50c710 refactor: move `CConnman::`(`Un`)`registerEvents` to ETE (Kittywhiskers Van Gogh)
3a9f386 refactor: move `SOCKET` addition/removal from interest list to ETE (Kittywhiskers Van Gogh)
212df06 refactor: introduce `EdgeTriggeredEvents`, move {epoll, kqueue} fd there (Kittywhiskers Van Gogh)
3b11ef9 refactor: move `CConnman::SocketEventsMode` to `util/sock.h` (Kittywhiskers Van Gogh)
Pull request description:
## Motivation
`CConnman` is an entity that contains a lot of platform-specific implementation logic, both inherited from upstream and added upon by Dash (support for edge-triggered socket events modes like `epoll` on Linux and `kqueue` on FreeBSD/Darwin).
Bitcoin has since moved to strip down `CConnman` by moving peer-related logic to the `Peer` struct in `net_processing` (portions of which are backported in #5982 and friends, tracking efforts from bitcoin#19398) and moving socket-related logic to `Sock` (portions of which are aimed to be backported in #6004, tracking efforts from bitcoin#21878).
Due to the direction being taken and the difference in how edge-triggered events modes operate (utilizing interest lists and events instead of iterating over each socket) in comparison to level-triggered modes (which are inherited from upstream), it would be reasonable to therefore, isolate Dash-specific code into its own entities and minimize the information `CConnman` has about its internal workings.
One of the visible benefits of this approach is comparing `develop` (as of this writing, d44b0d5) and this pull request for interactions between wakeup pipes logic and {`epoll`, `kqueue`} logic.
This is what construction looks like:
https://github.com/dashpay/dash/blob/d44b0d5dcb9b54821d582b267a8b92264be2da1b/src/net.cpp#L3358-L3397
But, if we segment wakeup pipes logic (that work on any platform with POSIX APIs and excludes Windows) and {`epoll`, `kqueue`} logic (calling them `EdgeTriggeredEvents` instead), construction looks different:
https://github.com/dashpay/dash/blob/907a3515170abed4ce9018115ed591e6ca9f4800/src/util/wpipe.cpp#L12-L38
Now wakeup pipes logic doesn't need to know what socket events mode is being used nor are the implementation aspects of (de)registering it its concern, that is now `EdgeTriggeredEvents` problem.
## Additional Information
* This pull request will need testing on macOS (FreeBSD isn't a tier-one target) to ensure that lack of breakage in `kqueue`-specific logic.
## Breaking Changes
* Dependency for #6018
* More logging has been introduced and existing log messages have been made more exhaustive. If there is parsing that relies on a particular template, they will have to be updated.
* If `EdgeTriggeredEvents` or `WakeupPipes` fail to initialize or are incorrectly initialized and not destroyed immediately, any further attempts at calling any of its functions will result in an `assert`-induced crash. Earlier behavior may have allowed for silent failure but segmentation of logic from `CConnman` means the newly created instances must only exist if the circumstances needed for it to initialize correctly are present.
This is to ensure that `CConnman` doesn't have to concern itself with internal workings of either entities.
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)**
- [x] I have made corresponding changes to the documentation **(note: N/A)**
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
PastaPastaPasta:
utACK bd8b5d4
Tree-SHA512: 8f793d4b4f2d8091e05bb9cc108013e924bbfbf19081290d9c0dfd91b0f2c80652ccf853f1596562942b4433509149c526e111396937988db605707ae1fe7366epoll, kqueue} (de)init logic and wakeup pipes logic out of CConnman and into EdgeTriggeredEvents and WakeupPipes
10 files changed
+600
-312
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
327 | 327 | | |
328 | 328 | | |
329 | 329 | | |
| 330 | + | |
330 | 331 | | |
331 | 332 | | |
332 | 333 | | |
| |||
362 | 363 | | |
363 | 364 | | |
364 | 365 | | |
| 366 | + | |
365 | 367 | | |
366 | 368 | | |
367 | 369 | | |
| |||
776 | 778 | | |
777 | 779 | | |
778 | 780 | | |
| 781 | + | |
779 | 782 | | |
780 | 783 | | |
781 | 784 | | |
| |||
795 | 798 | | |
796 | 799 | | |
797 | 800 | | |
| 801 | + | |
798 | 802 | | |
799 | 803 | | |
800 | 804 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2515 | 2515 | | |
2516 | 2516 | | |
2517 | 2517 | | |
2518 | | - | |
2519 | | - | |
2520 | | - | |
2521 | | - | |
2522 | | - | |
2523 | | - | |
2524 | | - | |
2525 | | - | |
2526 | | - | |
2527 | | - | |
2528 | | - | |
2529 | | - | |
2530 | | - | |
2531 | | - | |
2532 | | - | |
2533 | | - | |
2534 | | - | |
| 2518 | + | |
| 2519 | + | |
| 2520 | + | |
2535 | 2521 | | |
2536 | 2522 | | |
2537 | 2523 | | |
| |||
0 commit comments