From df74a9222f4e40d7b9ba41c77d443490c97fd333 Mon Sep 17 00:00:00 2001 From: Robin Burchell Date: Sat, 18 May 2019 02:00:01 +0200 Subject: [PATCH] Kernel: Fix timeout support in select The scheduler expects m_select_timeout to act as a deadline. That is, it should contain the time that a task should wake at -- but we were directly copying the time from userspace, which meant that it always returned virtually immediately. At the same time, fix CEventLoop to not rely on the broken select behavior by subtracting the current time from the time of the nearest timer. --- AK/Time.h | 25 +++++++++++++++++++++++++ Kernel/Process.cpp | 6 +++++- LibCore/CElapsedTimer.cpp | 13 ++----------- LibCore/CEventLoop.cpp | 11 ++++++++--- 4 files changed, 40 insertions(+), 15 deletions(-) create mode 100644 AK/Time.h diff --git a/AK/Time.h b/AK/Time.h new file mode 100644 index 00000000000..3839f18f176 --- /dev/null +++ b/AK/Time.h @@ -0,0 +1,25 @@ +#pragma once + +namespace AK { + +inline void timeval_sub(const struct timeval* a, const struct timeval* b, struct timeval* result) +{ + result->tv_sec = a->tv_sec - b->tv_sec; + result->tv_usec = a->tv_usec - b->tv_usec; + if (result->tv_usec < 0) { + --result->tv_sec; + result->tv_usec += 1000000; + } +} + +inline void timeval_add(const struct timeval* a, const struct timeval* b, struct timeval* result) +{ + result->tv_sec = a->tv_sec + b->tv_sec; + result->tv_usec = a->tv_usec + b->tv_usec; + if (result->tv_usec > 1000000) { + ++result->tv_sec; + result->tv_usec -= 1000000; + } +} + +} diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 4063a2263ba..ea5cd649354 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -1735,7 +1736,9 @@ int Process::sys$select(const Syscall::SC_select_params* params) (void)exceptfds; if (timeout) { - current->m_select_timeout = *timeout; + struct timeval now; + kgettimeofday(now); + AK::timeval_add(&now, timeout, ¤t->m_select_timeout); current->m_select_has_timeout = true; } else { current->m_select_has_timeout = false; @@ -1829,6 +1832,7 @@ int Process::sys$poll(pollfd* fds, int nfds, int timeout) current->m_select_write_fds.append(fds[i].fd); } + // FIXME: this should set m_select_timeout, right? if (timeout < 0) current->block(Thread::State::BlockedSelect); diff --git a/LibCore/CElapsedTimer.cpp b/LibCore/CElapsedTimer.cpp index b156147a8b9..aec3658fc44 100644 --- a/LibCore/CElapsedTimer.cpp +++ b/LibCore/CElapsedTimer.cpp @@ -1,5 +1,6 @@ #include #include +#include #include void CElapsedTimer::start() @@ -8,22 +9,12 @@ void CElapsedTimer::start() gettimeofday(&m_start_time, nullptr); } -inline void timersub(const struct timeval* a, const struct timeval* b, struct timeval* result) -{ - result->tv_sec = a->tv_sec - b->tv_sec; - result->tv_usec = a->tv_usec - b->tv_usec; - if (result->tv_usec < 0) { - --result->tv_sec; - result->tv_usec += 1000000; - } -} - int CElapsedTimer::elapsed() const { ASSERT(is_valid()); struct timeval now; gettimeofday(&now, nullptr); struct timeval diff; - timersub(&now, &m_start_time, &diff); + AK::timeval_sub(&now, &m_start_time, &diff); return diff.tv_sec * 1000 + diff.tv_usec / 1000; } diff --git a/LibCore/CEventLoop.cpp b/LibCore/CEventLoop.cpp index 614e8311167..7700a86494c 100644 --- a/LibCore/CEventLoop.cpp +++ b/LibCore/CEventLoop.cpp @@ -14,6 +14,7 @@ #include #include #include +#include //#define CEVENTLOOP_DEBUG //#define DEFERRED_INVOKE_DEBUG @@ -179,18 +180,22 @@ void CEventLoop::wait_for_event() queued_events_is_empty = m_queued_events.is_empty(); } + timeval now; struct timeval timeout = { 0, 0 }; - if (!s_timers->is_empty() && queued_events_is_empty) + if (!s_timers->is_empty() && queued_events_is_empty) { + gettimeofday(&now, nullptr); get_next_timer_expiration(timeout); + AK::timeval_sub(&timeout, &now, &timeout); + } int rc = select(max_fd + 1, &rfds, &wfds, nullptr, (queued_events_is_empty && s_timers->is_empty()) ? nullptr : &timeout); if (rc < 0) { ASSERT_NOT_REACHED(); } - timeval now; - if (!s_timers->is_empty()) + if (!s_timers->is_empty()) { gettimeofday(&now, nullptr); + } for (auto& it : *s_timers) { auto& timer = *it.value;