Skip to content

Commit 0342f3b

Browse files
committed
client: handle timeout more safely
Currently all wait methods are just cycling in a loop while timer is not expired. At the beginning of each iteration, `timeout - timer.elapsed()` expression is used to calculate timeout for `NetProvider::wait()`, and that `wait` expects only non-negative timeouts (or `-1` for infinity). So, for example, if the operating system will reschedule the process right between `timer.isExpired()` and `timer.elapsed()` call, timer can become expired after we are scheduled again, and we will suddenly pass a negative value to the provider. Let's replace `elapsed` method with more safe `timeLeft` which returns zero when the timeout is expired. Also, such method will be handy in the following commits. Closes #128
1 parent 7fc1912 commit 0342f3b

File tree

2 files changed

+17
-7
lines changed

2 files changed

+17
-7
lines changed

src/Client/Connector.hpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ Connector<BUFFER, NetProvider>::wait(Connection<BUFFER, NetProvider> &conn,
187187
return -1;
188188
while (!conn.hasError() && !conn.futureIsReady(future) &&
189189
!timer.isExpired()) {
190-
if (m_NetProvider.wait(timeout - timer.elapsed()) != 0) {
190+
if (m_NetProvider.wait(timer.timeLeft()) != 0) {
191191
conn.setError(std::string("Failed to poll: ") +
192192
strerror(errno), errno);
193193
return -1;
@@ -227,7 +227,7 @@ Connector<BUFFER, NetProvider>::waitAll(Connection<BUFFER, NetProvider> &conn,
227227
timer.start();
228228
size_t last_not_ready = 0;
229229
while (!conn.hasError() && !timer.isExpired()) {
230-
if (m_NetProvider.wait(timeout - timer.elapsed()) != 0) {
230+
if (m_NetProvider.wait(timer.timeLeft()) != 0) {
231231
conn.setError(std::string("Failed to poll: ") +
232232
strerror(errno), errno);
233233
return -1;
@@ -265,7 +265,7 @@ Connector<BUFFER, NetProvider>::waitAny(int timeout)
265265
Timer timer{timeout};
266266
timer.start();
267267
while (m_ReadyToDecode.empty() && !timer.isExpired())
268-
m_NetProvider.wait(timeout - timer.elapsed());
268+
m_NetProvider.wait(timer.timeLeft());
269269
if (m_ReadyToDecode.empty()) {
270270
LOG_ERROR("wait() has been timed out! No responses are received");
271271
return std::nullopt;
@@ -288,7 +288,7 @@ Connector<BUFFER, NetProvider>::waitCount(Connection<BUFFER, NetProvider> &conn,
288288
timer.start();
289289
size_t ready_futures = conn.getFutureCount();
290290
while (!conn.hasError() && !timer.isExpired()) {
291-
if (m_NetProvider.wait(timeout - timer.elapsed()) != 0) {
291+
if (m_NetProvider.wait(timer.timeLeft()) != 0) {
292292
conn.setError(std::string("Failed to poll: ") +
293293
strerror(errno), errno);
294294
return -1;

src/Utils/Timer.hpp

+13-3
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
* THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
3030
* SUCH DAMAGE.
3131
*/
32+
#include <algorithm>
3233
#include <chrono>
3334

3435
class Timer {
@@ -48,13 +49,22 @@ class Timer {
4849
std::chrono::duration_cast<std::chrono::milliseconds>(end - m_Start);
4950
return elapsed >= m_Timeout;
5051
}
51-
int elapsed() const
52+
/**
53+
* The function to obtain amount of time left. Returns:
54+
* 1. `-1` if the initial timeout was `-1`.
55+
* 2. `0` if the timer has expired.
56+
* 3. Otherwise, amount of milliseconds left is returned.
57+
* NB: the function should not be used for expiration check - use `isExpired` instead.
58+
*/
59+
int timeLeft() const
5260
{
5361
if (m_Timeout == std::chrono::milliseconds{-1})
54-
return 0;
62+
return -1;
5563
std::chrono::time_point<std::chrono::steady_clock> end =
5664
std::chrono::steady_clock::now();
57-
return std::chrono::duration_cast<std::chrono::milliseconds>(end - m_Start).count();
65+
int timeLeft = m_Timeout.count() -
66+
std::chrono::duration_cast<std::chrono::milliseconds>(end - m_Start).count();
67+
return std::max(0, timeLeft);
5868
}
5969
private:
6070
std::chrono::milliseconds m_Timeout;

0 commit comments

Comments
 (0)