refactor: move {epoll, kqueue} (de)init logic and wakeup pipes logic out of CConnman and into EdgeTriggeredEvents and WakeupPipes

[kwvg]

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:

dash/src/net.cpp

Lines 3358 to 3397 in d44b0d5

	#ifdef USE_WAKEUP_PIPE
	if (pipe(wakeupPipe) != 0) {
	wakeupPipe[0] = wakeupPipe[1] = -1;
	LogPrint(BCLog::NET, "pipe() for wakeupPipe failed\n");
	} else {
	int fFlags = fcntl(wakeupPipe[0], F_GETFL, 0);
	if (fcntl(wakeupPipe[0], F_SETFL, fFlags | O_NONBLOCK) == -1) {
	LogPrint(BCLog::NET, "fcntl for O_NONBLOCK on wakeupPipe failed\n");
	}
	fFlags = fcntl(wakeupPipe[1], F_GETFL, 0);
	if (fcntl(wakeupPipe[1], F_SETFL, fFlags | O_NONBLOCK) == -1) {
	LogPrint(BCLog::NET, "fcntl for O_NONBLOCK on wakeupPipe failed\n");
	}
	#ifdef USE_KQUEUE
	if (socketEventsMode == SOCKETEVENTS_KQUEUE) {
	struct kevent event;
	EV_SET(&event, wakeupPipe[0], EVFILT_READ, EV_ADD, 0, 0, nullptr);
	int r = kevent(kqueuefd, &event, 1, nullptr, 0, nullptr);
	if (r != 0) {
	LogPrint(BCLog::NET, "%s -- kevent(%d, %d, %d, ...) failed. error: %s\n", __func__,
	kqueuefd, EV_ADD, wakeupPipe[0], NetworkErrorString(WSAGetLastError()));
	return false;
	}
	}
	#endif
	#ifdef USE_EPOLL
	if (socketEventsMode == SOCKETEVENTS_EPOLL) {
	epoll_event event;
	event.events = EPOLLIN;
	event.data.fd = wakeupPipe[0];
	int r = epoll_ctl(epollfd, EPOLL_CTL_ADD, wakeupPipe[0], &event);
	if (r != 0) {
	LogPrint(BCLog::NET, "%s -- epoll_ctl(%d, %d, %d, ...) failed. error: %s\n", __func__,
	epollfd, EPOLL_CTL_ADD, wakeupPipe[0], NetworkErrorString(WSAGetLastError()));
	return false;
	}
	}
	#endif
	}
	#endif

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:

dash/src/util/wpipe.cpp

Lines 12 to 38 in 907a351

	WakeupPipe::WakeupPipe(EdgeTriggeredEvents* edge_trig_events)
	: m_edge_trig_events{edge_trig_events}
	{
	#ifdef USE_WAKEUP_PIPE
	if (pipe(m_pipe.data()) != 0) {
	LogPrintf("Unable to initialize WakeupPipe, pipe() for m_pipe failed with error %s\n",
	NetworkErrorString(WSAGetLastError()));
	return;
	}
	for (size_t idx = 0; idx < m_pipe.size(); idx++) {
	int flags = fcntl(m_pipe[idx], F_GETFL, 0);
	if (fcntl(m_pipe[idx], F_SETFL, flags | O_NONBLOCK) == -1) {
	LogPrintf("Unable to initialize WakeupPipe, fcntl for O_NONBLOCK on m_pipe[%d] failed with error %s\n", idx,
	NetworkErrorString(WSAGetLastError()));
	return;
	}
	}
	if (edge_trig_events && !edge_trig_events->RegisterPipe(m_pipe[0])) {
	LogPrintf("Unable to initialize WakeupPipe, EdgeTriggeredEvents::RegisterPipe() failed for m_pipe[0] = %d\n",
	m_pipe[0]);
	return;
	}
	m_valid = true;
	#else
	LogPrintf("Attempting to initialize WakeupPipe without support compiled in!\n");
	#endif /* USE_WAKEUP_PIPE */
	}

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 refactor: backport bitcoin#24356, extend -socketevents to Sock, implement Sock::WaitMany{Epoll, KQueue}  #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:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests (note: N/A)
• I have made corresponding changes to the documentation (note: N/A)
• I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

[MrDefacto]

I can do FreeBSD testing. I know that there are a couple hundred masternodes running on FreeBSD. I don't think it's unimportant.

[UdjinM6]

works with no issues on mac (kqueue mode) it seems, a couple of suggestions: e0deb9e and maybe 4e7f80e

[kwvg]

@MrDefacto Messaged you on Keybase!

[kwvg]

a couple of suggestions: e0deb9e and maybe 4e7f80e

With regards to e0deb9e , I was originally planning to move SocketEventsMode to util/sock.h in net_processing_6 (see kwvg@ed1409d) but getting rid of EdgeEventsMode and the need to sync enums was a good enough to reason to cherry-pick it. Thanks for the suggestion!

As for 4e7f80e, I'm leaning in the opposite direction and attempting to limit CConnman's knowledge of the macro (this PR still has a few uses of USE_{EPOLL, KQUEUE, POLL} that is done away with in the successor PR). The latest push (d5f0838) removes a few more USE_WAKEUP_PIPE usage and eventually am hoping to contain it to util/wpipes.cpp entirely.

---

Also, a condition inversion in WakeupPipe::Drain() has been resolved in the latest push (for some reason the infinite loop introduced didn't manifest in this PR but did in net_processing_6).

[github-actions]

This pull request has conflicts, please rebase.

[UdjinM6]

LGTM, light ACK (on mac)

[PastaPastaPasta]

why not use something like __builtin_unreachable?

[kwvg]

__builtin_unreachable is a compiler hint to optimize it out but is within the realm of possibility that the path might be taken due to erroneous code. As it is not an assertion that the code path isn't taken, assert seemed more suitable for the job.

[PastaPastaPasta]

why not use something like __builtin_unreachable?

[PastaPastaPasta]

utACK bd8b5d4