Skip to content

Commit

Permalink
Have Linux/MdnsImpl use System::Layer timer. (#9537)
Browse files Browse the repository at this point in the history
#### Problem

Linux/MdnsImpl currently uses ad-hoc coupling with the POSIX I/O event
loop, which intrudes on and constrains the latter's implementation.

#### Change overview

Replace the `GetMdnsTimeout()`/`HandleMdnsTimeout()` pair with a
`System::Layer` timer tracking the earliest mDNS timeout.

The last remnants of ~the Old Republic~ `select()` have been swept away.

#### Testing

Existing unit test TestMdns
  • Loading branch information
kpschoedel authored Sep 13, 2021
1 parent e90a2b3 commit 016b1d5
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 78 deletions.
5 changes: 0 additions & 5 deletions src/controller/java/MdnsImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,6 @@ CHIP_ERROR ChipMdnsResolve(MdnsService * service, Inet::InterfaceId interface, M
return CHIP_NO_ERROR;
}

// Implementation of other methods required by the CHIP stack

void GetMdnsTimeout(timeval & timeout) {}
void HandleMdnsTimeout() {}

// Implemention of Java-specific functions

void InitializeWithObjects(jobject resolverObject, jobject mdnsCallbackObject)
Expand Down
4 changes: 0 additions & 4 deletions src/platform/Darwin/MdnsImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -549,9 +549,5 @@ CHIP_ERROR ChipMdnsResolve(MdnsService * service, chip::Inet::InterfaceId interf
return Resolve(context, callback, interfaceId, service->mAddressType, regtype.c_str(), service->mName);
}

void GetMdnsTimeout(timeval & timeout) {}

void HandleMdnsTimeout() {}

} // namespace Mdns
} // namespace chip
69 changes: 31 additions & 38 deletions src/platform/Linux/MdnsImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include <lib/support/CHIPMemString.h>
#include <lib/support/CodeUtils.h>
#include <platform/internal/CHIPDeviceLayerInternal.h>
#include <system/SystemLayer.h>

using chip::Mdns::kMdnsTypeMaxSize;
using chip::Mdns::MdnsServiceProtocol;
Expand Down Expand Up @@ -136,8 +137,6 @@ namespace Mdns {

MdnsAvahi MdnsAvahi::sInstance;

constexpr uint64_t kUsPerSec = 1000 * 1000;

Poller::Poller()
{
mAvahiPoller.userdata = this;
Expand All @@ -149,6 +148,8 @@ Poller::Poller()
mAvahiPoller.timeout_new = TimeoutNew;
mAvahiPoller.timeout_update = TimeoutUpdate;
mAvahiPoller.timeout_free = TimeoutFree;

mEarliestTimeout = std::chrono::steady_clock::time_point();
}

AvahiWatch * Poller::WatchNew(const struct AvahiPoll * poller, int fd, AvahiWatchEvent event, AvahiWatchCallback callback,
Expand Down Expand Up @@ -237,9 +238,10 @@ steady_clock::time_point GetAbsTimeout(const struct timeval * timeout)

AvahiTimeout * Poller::TimeoutNew(const struct timeval * timeout, AvahiTimeoutCallback callback, void * context)
{

mTimers.emplace_back(new AvahiTimeout{ GetAbsTimeout(timeout), callback, timeout != nullptr, context, this });
return mTimers.back().get();
AvahiTimeout * timer = mTimers.back().get();
SystemTimerUpdate(timer);
return timer;
}

void Poller::TimeoutUpdate(AvahiTimeout * timer, const struct timeval * timeout)
Expand All @@ -248,6 +250,7 @@ void Poller::TimeoutUpdate(AvahiTimeout * timer, const struct timeval * timeout)
{
timer->mAbsTimeout = GetAbsTimeout(timeout);
timer->mEnabled = true;
static_cast<Poller *>(timer->mPoller)->SystemTimerUpdate(timer);
}
else
{
Expand All @@ -267,48 +270,48 @@ void Poller::TimeoutFree(AvahiTimeout & timer)
mTimers.end());
}

void Poller::GetTimeout(timeval & timeout)
void Poller::SystemTimerCallback(System::Layer * layer, void * data)
{
microseconds timeoutVal = seconds(timeout.tv_sec) + microseconds(timeout.tv_usec);
static_cast<Poller *>(data)->HandleTimeout();
}

void Poller::HandleTimeout()
{
mEarliestTimeout = std::chrono::steady_clock::time_point();
steady_clock::time_point now = steady_clock::now();

AvahiTimeout * earliest = nullptr;
for (auto && timer : mTimers)
{
steady_clock::time_point absTimeout = timer->mAbsTimeout;
steady_clock::time_point now = steady_clock::now();

if (!timer->mEnabled)
{
continue;
}
if (absTimeout < now)
if (timer->mAbsTimeout <= now)
{
timeoutVal = microseconds(0);
break;
timer->mCallback(timer.get(), timer->mContext);
}
else
{
timeoutVal = std::min(timeoutVal, duration_cast<microseconds>(absTimeout - now));
if ((earliest == nullptr) || (timer->mAbsTimeout < earliest->mAbsTimeout))
{
earliest = timer.get();
}
}
}

timeout.tv_sec = static_cast<uint64_t>(timeoutVal.count()) / kUsPerSec;
timeout.tv_usec = static_cast<uint64_t>(timeoutVal.count()) % kUsPerSec;
if (earliest)
{
SystemTimerUpdate(earliest);
}
}

void Poller::HandleTimeout()
void Poller::SystemTimerUpdate(AvahiTimeout * timer)
{
steady_clock::time_point now = steady_clock::now();

for (auto && timer : mTimers)
if ((mEarliestTimeout == std::chrono::steady_clock::time_point()) || (timer->mAbsTimeout < mEarliestTimeout))
{
if (!timer->mEnabled)
{
continue;
}
if (timer->mAbsTimeout <= now)
{
timer->mCallback(timer.get(), timer->mContext);
}
mEarliestTimeout = timer->mAbsTimeout;
auto msDelay = std::chrono::duration_cast<std::chrono::milliseconds>(steady_clock::now() - mEarliestTimeout).count();
DeviceLayer::SystemLayer.StartTimer(msDelay, SystemTimerCallback, this);
}
}

Expand Down Expand Up @@ -751,16 +754,6 @@ void MdnsAvahi::HandleResolve(AvahiServiceResolver * resolver, AvahiIfIndex inte
chip::Platform::Delete(context);
}

void GetMdnsTimeout(timeval & timeout)
{
MdnsAvahi::GetInstance().GetPoller().GetTimeout(timeout);
}

void HandleMdnsTimeout()
{
MdnsAvahi::GetInstance().GetPoller().HandleTimeout();
}

CHIP_ERROR ChipMdnsInit(MdnsAsyncReturnCallback initCallback, MdnsAsyncReturnCallback errorCallback, void * context)
{
return MdnsAvahi::GetInstance().Init(initCallback, errorCallback, context);
Expand Down
6 changes: 5 additions & 1 deletion src/platform/Linux/MdnsImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ class Poller
public:
Poller(void);

void GetTimeout(timeval & timeout);
void HandleTimeout();

const AvahiPoll * GetAvahiPoll(void) const { return &mAvahiPoller; }
Expand All @@ -88,8 +87,13 @@ class Poller
static void TimeoutFree(AvahiTimeout * timer);
void TimeoutFree(AvahiTimeout & timer);

void SystemTimerUpdate(AvahiTimeout * timer);
static void SystemTimerCallback(System::Layer * layer, void * data);

std::vector<std::unique_ptr<AvahiWatch>> mWatches;
std::vector<std::unique_ptr<AvahiTimeout>> mTimers;
std::chrono::steady_clock::time_point mEarliestTimeout;

AvahiPoll mAvahiPoller;
};

Expand Down
11 changes: 0 additions & 11 deletions src/system/SystemLayerImplLibevent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,6 @@
#include <fcntl.h>
#include <unistd.h>

#if CHIP_DEVICE_CONFIG_ENABLE_MDNS && !__ZEPHYR__

namespace chip {
namespace Mdns {
void GetMdnsTimeout(timeval & timeout);
void HandleMdnsTimeout();
} // namespace Mdns
} // namespace chip

#endif // CHIP_DEVICE_CONFIG_ENABLE_MDNS && !__ZEPHYR__

#ifndef CHIP_CONFIG_LIBEVENT_DEBUG_CHECKS
#define CHIP_CONFIG_LIBEVENT_DEBUG_CHECKS 1
#endif
Expand Down
19 changes: 0 additions & 19 deletions src/system/SystemLayerImplSelect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,6 @@
#define PTHREAD_NULL 0
#endif // CHIP_SYSTEM_CONFIG_POSIX_LOCKING && !defined(PTHREAD_NULL)

#if CHIP_DEVICE_CONFIG_ENABLE_MDNS && !__ZEPHYR__

namespace chip {
namespace Mdns {
void GetMdnsTimeout(timeval & timeout);
void HandleMdnsTimeout();
} // namespace Mdns
} // namespace chip

#endif // CHIP_DEVICE_CONFIG_ENABLE_MDNS && !__ZEPHYR__

namespace chip {
namespace System {

Expand Down Expand Up @@ -351,10 +340,6 @@ void LayerImplSelect::PrepareEvents()
const Clock::MonotonicMilliseconds sleepTime = (awakenTime > currentTime) ? (awakenTime - currentTime) : 0;
Clock::MillisecondsToTimeval(sleepTime, mNextTimeout);

#if CHIP_DEVICE_CONFIG_ENABLE_MDNS && !__ZEPHYR__ && !__MBED__
chip::Mdns::GetMdnsTimeout(mNextTimeout);
#endif // CHIP_DEVICE_CONFIG_ENABLE_MDNS && !__ZEPHYR__

mMaxFd = -1;
FD_ZERO(&mSelected.mReadSet);
FD_ZERO(&mSelected.mWriteSet);
Expand Down Expand Up @@ -419,10 +404,6 @@ void LayerImplSelect::HandleEvents()
}
}

#if CHIP_DEVICE_CONFIG_ENABLE_MDNS && !__ZEPHYR__ && !__MBED__
chip::Mdns::HandleMdnsTimeout();
#endif // CHIP_DEVICE_CONFIG_ENABLE_MDNS && !__ZEPHYR__

#if CHIP_SYSTEM_CONFIG_POSIX_LOCKING
mHandleSelectThread = PTHREAD_NULL;
#endif // CHIP_SYSTEM_CONFIG_POSIX_LOCKING
Expand Down

0 comments on commit 016b1d5

Please sign in to comment.