Skip to content

Commit

Permalink
SystemLayerImplSelect: libev: eliminate I/O wakeup thread
Browse files Browse the repository at this point in the history
libev based regular builds do not need a separate I/O wakeup thread, so
the wakeup thread can be eliminated in CHIP_SYSTEM_CONFIG_USE_LIBEV case.

In normal operation, libev based builds also never call Signal(), so if it
is still called it now emits a error log message, to indicate something might
be wrong in the setup.

However we keep Signal() and related LayerSocketsLoop methods, following the
Darwin dispatch implementation, as fallback to select-based event handling
seems to be needed for some I/O tests.

As noted in a comment to the original libev PR [1], eventually, the select() based
mainloop and external mainloop based solutions like Darwin Dispatch and libev
should be detangled and extracted into separate classes, adapting all the tests
that somehow rely on the select() fallback. As a single self-funded
developer however, I cannot possibly be expected to solve this for Apple ;-)

[1] project-chip#24232 (review)
  • Loading branch information
plan44 committed Aug 29, 2023
1 parent aa14260 commit 3081f7a
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 4 deletions.
11 changes: 11 additions & 0 deletions src/system/SystemLayerImplSelect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,10 @@ CHIP_ERROR LayerImplSelect::Init()
mHandleSelectThread = PTHREAD_NULL;
#endif // CHIP_SYSTEM_CONFIG_POSIX_LOCKING

#if !CHIP_SYSTEM_CONFIG_USE_LIBEV
// Create an event to allow an arbitrary thread to wake the thread in the select loop.
ReturnErrorOnFailure(mWakeEvent.Open(*this));
#endif // !CHIP_SYSTEM_CONFIG_USE_LIBEV

VerifyOrReturnError(mLayerState.SetInitialized(), CHIP_ERROR_INCORRECT_STATE);
return CHIP_NO_ERROR;
Expand Down Expand Up @@ -113,13 +115,18 @@ void LayerImplSelect::Shutdown()
mTimerPool.ReleaseAll();
#endif // CHIP_SYSTEM_CONFIG_USE_DISPATCH/LIBEV

#if !CHIP_SYSTEM_CONFIG_USE_LIBEV
mWakeEvent.Close(*this);
#endif // !CHIP_SYSTEM_CONFIG_USE_LIBEV

mLayerState.ResetFromShuttingDown(); // Return to uninitialized state to permit re-initialization.
}

void LayerImplSelect::Signal()
{
#if CHIP_SYSTEM_CONFIG_USE_LIBEV
ChipLogError(DeviceLayer, "Signal() should not be called in CHIP_SYSTEM_CONFIG_USE_LIBEV builds (might be ok in tests)");
#else
/*
* Wake up the I/O thread by writing a single byte to the wake pipe.
*
Expand All @@ -143,6 +150,7 @@ void LayerImplSelect::Signal()

ChipLogError(chipSystemLayer, "System wake event notify failed: %" CHIP_ERROR_FORMAT, status.Format());
}
#endif // !CHIP_SYSTEM_CONFIG_USE_LIBEV
}

CHIP_ERROR LayerImplSelect::StartTimer(Clock::Timeout delay, TimerCompleteCallback onComplete, void * appState)
Expand Down Expand Up @@ -274,7 +282,10 @@ void LayerImplSelect::CancelTimer(TimerCompleteCallback onComplete, void * appSt
#endif // CHIP_SYSTEM_CONFIG_USE_DISPATCH/LIBEV

mTimerPool.Release(timer);
#if !CHIP_SYSTEM_CONFIG_USE_LIBEV
// LIBEV has no I/O wakeup thread, so must not call Signal()
Signal();
#endif
}

CHIP_ERROR LayerImplSelect::ScheduleWork(TimerCompleteCallback onComplete, void * appState)
Expand Down
2 changes: 2 additions & 0 deletions src/system/SystemLayerImplSelect.h
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,9 @@ class LayerImplSelect : public LayerSocketsLoop
int mSelectResult;

ObjectLifeCycle mLayerState;
#if !CHIP_SYSTEM_CONFIG_USE_LIBEV
WakeEvent mWakeEvent;
#endif

#if CHIP_SYSTEM_CONFIG_POSIX_LOCKING
std::atomic<pthread_t> mHandleSelectThread;
Expand Down
4 changes: 2 additions & 2 deletions src/system/WakeEvent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

#include <system/SystemConfig.h>

#if CHIP_SYSTEM_CONFIG_USE_SOCKETS
#if CHIP_SYSTEM_CONFIG_USE_SOCKETS && !CHIP_SYSTEM_CONFIG_USE_LIBEV

#include <system/WakeEvent.h>

Expand Down Expand Up @@ -207,4 +207,4 @@ CHIP_ERROR WakeEvent::Notify() const
} // namespace System
} // namespace chip

#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS
#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS && !CHIP_SYSTEM_CONFIG_USE_LIBEV
4 changes: 2 additions & 2 deletions src/system/WakeEvent.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
// Include configuration headers
#include <system/SystemConfig.h>

#if CHIP_SYSTEM_CONFIG_USE_SOCKETS
#if CHIP_SYSTEM_CONFIG_USE_SOCKETS && !CHIP_SYSTEM_CONFIG_USE_LIBEV

#include <lib/core/CHIPError.h>
#include <system/SocketEvents.h>
Expand Down Expand Up @@ -67,4 +67,4 @@ class WakeEvent
} // namespace System
} // namespace chip

#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS
#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS && !CHIP_SYSTEM_CONFIG_USE_LIBEV

0 comments on commit 3081f7a

Please sign in to comment.