From ceee7acf43464dbbcca7e05911e354532c5435f6 Mon Sep 17 00:00:00 2001 From: Ahmad Samir Date: Sat, 29 Apr 2023 15:07:14 +0300 Subject: [PATCH] qcore_unix: port qt_safe_poll to QDeadlineTimer Remove qt_poll_msecs() since the "forever" state can be simply expressed with a QDeadlineTimer::Forever arg, instead of passing a nullptr timespec, and the negative timeouts treated as "run forever" is also encapsulated by QDealineTimer. Use the QDealineTimer(qint64) constructor in the call sites where the timeout could be negative, so that it creates a Forever timer (the QDeadlineTimer(chrono::duration) constructor uses setRemainingTime(duration) which handles negative timeouts by creating expired timers). Remove qt_gettime() (and do_gettime()). Drive-by changes: - Fix a narrowing conversion warning, qt_make_pollfd() takes an int - Remove an unused include Change-Id: I096319af5e191e28c3d39295fb1aafe9d69841e6 Reviewed-by: Thiago Macieira --- src/corelib/io/qprocess_unix.cpp | 6 +- src/corelib/kernel/qcore_unix.cpp | 63 +++---------------- src/corelib/kernel/qcore_unix_p.h | 18 +----- src/corelib/kernel/qeventdispatcher_unix.cpp | 27 ++++---- src/network/socket/qlocalserver_unix.cpp | 3 +- src/network/socket/qlocalsocket_unix.cpp | 5 +- .../socket/qnativesocketengine_unix.cpp | 2 +- .../kernel/qeventloop/tst_qeventloop.cpp | 4 +- 8 files changed, 32 insertions(+), 96 deletions(-) diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp index 41fb08ad05f..a949a492cd8 100644 --- a/src/corelib/io/qprocess_unix.cpp +++ b/src/corelib/io/qprocess_unix.cpp @@ -221,7 +221,7 @@ QProcessPoller::QProcessPoller(const QProcessPrivate &proc) int QProcessPoller::poll(const QDeadlineTimer &deadline) { - return qt_poll_msecs(pfds, n_pfds, deadline.remainingTime()); + return qt_safe_poll(pfds, n_pfds, deadline); } struct QChildProcess @@ -1092,15 +1092,15 @@ void QProcessPrivate::killProcess() bool QProcessPrivate::waitForStarted(const QDeadlineTimer &deadline) { - const qint64 msecs = deadline.remainingTime(); #if defined (QPROCESS_DEBUG) + const qint64 msecs = deadline.remainingTime(); qDebug("QProcessPrivate::waitForStarted(%lld) waiting for child to start (fd = %d)", msecs, childStartedPipe[0]); #endif pollfd pfd = qt_make_pollfd(childStartedPipe[0], POLLIN); - if (qt_poll_msecs(&pfd, 1, msecs) == 0) { + if (qt_safe_poll(&pfd, 1, deadline) == 0) { setError(QProcess::Timedout); #if defined (QPROCESS_DEBUG) qDebug("QProcessPrivate::waitForStarted(%lld) == false (timed out)", msecs); diff --git a/src/corelib/kernel/qcore_unix.cpp b/src/corelib/kernel/qcore_unix.cpp index b4bdad1ebbd..cec1ab2a0b1 100644 --- a/src/corelib/kernel/qcore_unix.cpp +++ b/src/corelib/kernel/qcore_unix.cpp @@ -4,7 +4,6 @@ #include #include "qcore_unix_p.h" -#include "qelapsedtimer.h" #include @@ -65,48 +64,10 @@ int qt_open64(const char *pathname, int flags, mode_t mode) #ifndef QT_BOOTSTRAPPED -static inline void do_gettime(qint64 *sec, qint64 *frac) -{ - timespec ts; - clockid_t clk = CLOCK_REALTIME; -#if defined(CLOCK_MONOTONIC_RAW) - clk = CLOCK_MONOTONIC_RAW; -#elif defined(CLOCK_MONOTONIC) - clk = CLOCK_MONOTONIC; -#endif - - clock_gettime(clk, &ts); - *sec = ts.tv_sec; - *frac = ts.tv_nsec; -} - -// also used in qeventdispatcher_unix.cpp -struct timespec qt_gettime() noexcept -{ - qint64 sec, frac; - do_gettime(&sec, &frac); - - timespec tv; - tv.tv_sec = sec; - tv.tv_nsec = frac; - - return tv; -} - #if QT_CONFIG(poll_pollts) # define ppoll pollts #endif -static inline bool time_update(struct timespec *tv, const struct timespec &start, - const struct timespec &timeout) -{ - // clock source is (hopefully) monotonic, so we can recalculate how much timeout is left; - // if it isn't monotonic, we'll simply hope that it hasn't jumped, because we have no alternative - struct timespec now = qt_gettime(); - *tv = timeout + start - now; - return tv->tv_sec >= 0; -} - [[maybe_unused]] static inline int timespecToMillisecs(const struct timespec *ts) { @@ -141,31 +102,27 @@ static inline int qt_ppoll(struct pollfd *fds, nfds_t nfds, const struct timespe using select(2) where necessary. In that case, returns -1 and sets errno to EINVAL if passed any descriptor greater than or equal to FD_SETSIZE. */ -int qt_safe_poll(struct pollfd *fds, nfds_t nfds, const struct timespec *timeout_ts) +int qt_safe_poll(struct pollfd *fds, nfds_t nfds, QDeadlineTimer deadline) { - if (!timeout_ts) { + if (deadline.isForever()) { // no timeout -> block forever int ret; EINTR_LOOP(ret, qt_ppoll(fds, nfds, nullptr)); return ret; } - timespec start = qt_gettime(); - timespec timeout = *timeout_ts; - + using namespace std::chrono; + nanoseconds remaining = deadline.remainingTimeAsDuration(); // loop and recalculate the timeout as needed - forever { - const int ret = qt_ppoll(fds, nfds, &timeout); + do { + timespec ts = durationToTimespec(remaining); + const int ret = qt_ppoll(fds, nfds, &ts); if (ret != -1 || errno != EINTR) return ret; + remaining = deadline.remainingTimeAsDuration(); + } while (remaining > 0ns); - // recalculate the timeout - if (!time_update(&timeout, start, *timeout_ts)) { - // timeout during update - // or clock reset, fake timeout error - return 0; - } - } + return 0; } #endif // QT_BOOTSTRAPPED diff --git a/src/corelib/kernel/qcore_unix_p.h b/src/corelib/kernel/qcore_unix_p.h index 2d55b9dcdc4..cad1c4f54ee 100644 --- a/src/corelib/kernel/qcore_unix_p.h +++ b/src/corelib/kernel/qcore_unix_p.h @@ -20,6 +20,7 @@ #include #include "qatomic.h" #include "qbytearray.h" +#include "qdeadlinetimer.h" #ifndef Q_OS_UNIX # error "qcore_unix_p.h included on a non-Unix system" @@ -376,8 +377,6 @@ static inline pid_t qt_safe_waitpid(pid_t pid, int *status, int options) # define _POSIX_MONOTONIC_CLOCK -1 #endif -// in qelapsedtimer_mac.cpp or qtimestamp_unix.cpp -timespec qt_gettime() noexcept; QByteArray qt_readlink(const char *path); /* non-static */ @@ -395,20 +394,7 @@ inline bool qt_haveLinuxProcfs() #endif } -Q_CORE_EXPORT int qt_safe_poll(struct pollfd *fds, nfds_t nfds, const struct timespec *timeout_ts); - -static inline int qt_poll_msecs(struct pollfd *fds, nfds_t nfds, int timeout) -{ - timespec ts, *pts = nullptr; - - if (timeout >= 0) { - ts.tv_sec = timeout / 1000; - ts.tv_nsec = (timeout % 1000) * 1000 * 1000; - pts = &ts; - } - - return qt_safe_poll(fds, nfds, pts); -} +Q_CORE_EXPORT int qt_safe_poll(struct pollfd *fds, nfds_t nfds, QDeadlineTimer deadline); static inline struct pollfd qt_make_pollfd(int fd, short events) { diff --git a/src/corelib/kernel/qeventdispatcher_unix.cpp b/src/corelib/kernel/qeventdispatcher_unix.cpp index 61f7b1a6655..d0b47757bf9 100644 --- a/src/corelib/kernel/qeventdispatcher_unix.cpp +++ b/src/corelib/kernel/qeventdispatcher_unix.cpp @@ -428,20 +428,18 @@ bool QEventDispatcherUNIX::processEvents(QEventLoop::ProcessEventsFlags flags) if (d->interrupt.loadRelaxed()) return false; - // If canWait is true, and include_timers is false or there are no pending - // timers, call qt_safe_poll() with a nullptr so that it waits until there - // are events to process (see QEventLoop::WaitForMoreEvents). - timespec *tm = nullptr; - timespec wait_tm = { 0, 0 }; - - if (!canWait) { - tm = &wait_tm; - } else if (include_timers) { - std::optional msecs = d->timerList.timerWait(); - if (msecs) { - wait_tm = durationToTimespec(*msecs); - tm = &wait_tm; + QDeadlineTimer deadline; + if (canWait) { + if (include_timers) { + std::optional msecs = d->timerList.timerWait(); + deadline = msecs ? QDeadlineTimer{*msecs} + : QDeadlineTimer(QDeadlineTimer::Forever); + } else { + deadline = QDeadlineTimer(QDeadlineTimer::Forever); } + } else { + // Using the default-constructed `deadline`, which is already expired, + // ensures the code in the do-while loop in qt_safe_poll runs at least once. } d->pollfds.clear(); @@ -455,8 +453,7 @@ bool QEventDispatcherUNIX::processEvents(QEventLoop::ProcessEventsFlags flags) d->pollfds.append(d->threadPipe.prepare()); int nevents = 0; - - switch (qt_safe_poll(d->pollfds.data(), d->pollfds.size(), tm)) { + switch (qt_safe_poll(d->pollfds.data(), d->pollfds.size(), deadline)) { case -1: qErrnoWarning("qt_safe_poll"); if (QT_CONFIG(poll_exit_on_error)) diff --git a/src/network/socket/qlocalserver_unix.cpp b/src/network/socket/qlocalserver_unix.cpp index dcd001a2630..9aa9a5b86fb 100644 --- a/src/network/socket/qlocalserver_unix.cpp +++ b/src/network/socket/qlocalserver_unix.cpp @@ -279,8 +279,7 @@ void QLocalServerPrivate::_q_onNewConnection() void QLocalServerPrivate::waitForNewConnection(int msec, bool *timedOut) { pollfd pfd = qt_make_pollfd(listenSocket, POLLIN); - - switch (qt_poll_msecs(&pfd, 1, msec)) { + switch (qt_safe_poll(&pfd, 1, QDeadlineTimer(msec))) { case 0: if (timedOut) *timedOut = true; diff --git a/src/network/socket/qlocalsocket_unix.cpp b/src/network/socket/qlocalsocket_unix.cpp index 6e9133ad8f4..af0dc988afe 100644 --- a/src/network/socket/qlocalsocket_unix.cpp +++ b/src/network/socket/qlocalsocket_unix.cpp @@ -25,7 +25,6 @@ using namespace std::chrono_literals; #define QT_CONNECT_TIMEOUT 30000 - QT_BEGIN_NAMESPACE using namespace Qt::StringLiterals; @@ -593,9 +592,7 @@ bool QLocalSocket::waitForConnected(int msec) auto remainingTime = deadline.remainingTimeAsDuration(); do { - timespec ts = durationToTimespec(remainingTime); - const int result = qt_safe_poll(&pfd, 1, &ts); - + const int result = qt_safe_poll(&pfd, 1, deadline); if (result == -1) d->setErrorAndEmit(QLocalSocket::UnknownSocketError, "QLocalSocket::waitForConnected"_L1); diff --git a/src/network/socket/qnativesocketengine_unix.cpp b/src/network/socket/qnativesocketengine_unix.cpp index 87c74f7f854..9656a3cef63 100644 --- a/src/network/socket/qnativesocketengine_unix.cpp +++ b/src/network/socket/qnativesocketengine_unix.cpp @@ -1364,7 +1364,7 @@ int QNativeSocketEnginePrivate::nativeSelect(QDeadlineTimer deadline, bool check if (checkWrite) pfd.events |= POLLOUT; - const int ret = qt_poll_msecs(&pfd, 1, deadline.remainingTime()); + const int ret = qt_safe_poll(&pfd, 1, deadline); if (ret <= 0) return ret; diff --git a/tests/auto/corelib/kernel/qeventloop/tst_qeventloop.cpp b/tests/auto/corelib/kernel/qeventloop/tst_qeventloop.cpp index 171f95157d0..4ebeec60830 100644 --- a/tests/auto/corelib/kernel/qeventloop/tst_qeventloop.cpp +++ b/tests/auto/corelib/kernel/qeventloop/tst_qeventloop.cpp @@ -402,8 +402,8 @@ public slots: dataSent = serverSocket->waitForBytesWritten(-1); if (dataSent) { - pollfd pfd = qt_make_pollfd(socket->socketDescriptor(), POLLIN); - dataReadable = (1 == qt_safe_poll(&pfd, 1, nullptr)); + pollfd pfd = qt_make_pollfd(int(socket->socketDescriptor()), POLLIN); + dataReadable = (1 == qt_safe_poll(&pfd, 1, QDeadlineTimer::Forever)); } if (!dataReadable) {