2021-03-30 22:54:41 +02:00
|
|
|
/*
|
|
|
|
|
* Copyright (c) 2021 The WebRTC project authors. All Rights Reserved.
|
|
|
|
|
*
|
|
|
|
|
* Use of this source code is governed by a BSD-style license
|
|
|
|
|
* that can be found in the LICENSE file in the root of the source
|
|
|
|
|
* tree. An additional intellectual property rights grant can be found
|
|
|
|
|
* in the file PATENTS. All contributing project authors may
|
|
|
|
|
* be found in the AUTHORS file in the root of the source tree.
|
|
|
|
|
*/
|
|
|
|
|
#ifndef NET_DCSCTP_TIMER_FAKE_TIMEOUT_H_
|
|
|
|
|
#define NET_DCSCTP_TIMER_FAKE_TIMEOUT_H_
|
|
|
|
|
|
|
|
|
|
#include <cstdint>
|
|
|
|
|
#include <functional>
|
|
|
|
|
#include <limits>
|
|
|
|
|
#include <memory>
|
|
|
|
|
#include <utility>
|
|
|
|
|
#include <vector>
|
|
|
|
|
|
dcsctp: Expire timers just before triggering them
In real life, when a Timeout expires, the caller is supposed to call
DcSctpSocket::HandleTimeout directly, as the Timeout that just expired
is stopped (it just expired), but the Timer still believes it's running.
The system is not in a consistent state.
In tests, all timeouts were evaluated at the same time, which, if two
timeouts expired at the same time, would put them both as "not running",
and with their timers believing they were running. So if you would do
any operation on a timer whose timeout had just expired, the timeout
would assert saying that "you can't stop a stopped timeout" or similar.
This isn't relevant in non-test scenarios.
Solved by expiring timeouts one by one.
Bug: webrtc:12614
Change-Id: I79d006f4d3e96854d77cec3eb0080aa23b8569cb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217560
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33925}
2021-05-05 14:00:50 +02:00
|
|
|
#include "absl/types/optional.h"
|
2022-01-26 18:38:13 +01:00
|
|
|
#include "api/task_queue/task_queue_base.h"
|
2021-03-30 22:54:41 +02:00
|
|
|
#include "net/dcsctp/public/timeout.h"
|
dcsctp: Handle starting timer from timer callback
This was caught in an integration test which had stricter assertions
than the FakeTimeout which is used in unit tests, so the first thing was
to add the same assertions to the FakeTimeout.
The issue is that when a Timer triggers, and if it's set to
automatically restart (possibly with an exponential backoff), the
`is_running_` field was set to true while the timer callback was called
to allow the client to know that the timer is in fact running, but the
timer was actually not started until the callback returned. Which made
sense, as the callback can with its return value override the duration,
which should affect the backoff algorithm.
The problem was when a timer was manually started within the callback.
As the Timer itself thought that it was already running, it first would
Stop() the underlying Timeout, then Start(). But calling Stop() on a
timeout that is not running is illegal, which set of assertions.
So the solution is to don't lie; Don't say that a timer is running when
it's not. Make sure that the timer is running when the timer callback is
triggered, which makes it consistent at all times. That may result in
unnecessary timeout invocations (stopping and starting), but that's not
too expensive.
Bug: webrtc:12614
Change-Id: I7b4447ccd88bd43d181e158f0d29b0770c8a3fd6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217522
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33926}
2021-05-05 12:36:52 +02:00
|
|
|
#include "rtc_base/checks.h"
|
2021-08-18 15:22:42 +02:00
|
|
|
#include "rtc_base/containers/flat_set.h"
|
2021-03-30 22:54:41 +02:00
|
|
|
|
|
|
|
|
namespace dcsctp {
|
|
|
|
|
|
|
|
|
|
// A timeout used in tests.
|
|
|
|
|
class FakeTimeout : public Timeout {
|
|
|
|
|
public:
|
2022-01-26 18:38:13 +01:00
|
|
|
FakeTimeout(std::function<TimeMs()> get_time,
|
|
|
|
|
std::function<void(FakeTimeout*)> on_delete)
|
2021-03-30 22:54:41 +02:00
|
|
|
: get_time_(std::move(get_time)), on_delete_(std::move(on_delete)) {}
|
|
|
|
|
|
|
|
|
|
~FakeTimeout() override { on_delete_(this); }
|
|
|
|
|
|
|
|
|
|
void Start(DurationMs duration_ms, TimeoutID timeout_id) override {
|
dcsctp: Handle starting timer from timer callback
This was caught in an integration test which had stricter assertions
than the FakeTimeout which is used in unit tests, so the first thing was
to add the same assertions to the FakeTimeout.
The issue is that when a Timer triggers, and if it's set to
automatically restart (possibly with an exponential backoff), the
`is_running_` field was set to true while the timer callback was called
to allow the client to know that the timer is in fact running, but the
timer was actually not started until the callback returned. Which made
sense, as the callback can with its return value override the duration,
which should affect the backoff algorithm.
The problem was when a timer was manually started within the callback.
As the Timer itself thought that it was already running, it first would
Stop() the underlying Timeout, then Start(). But calling Stop() on a
timeout that is not running is illegal, which set of assertions.
So the solution is to don't lie; Don't say that a timer is running when
it's not. Make sure that the timer is running when the timer callback is
triggered, which makes it consistent at all times. That may result in
unnecessary timeout invocations (stopping and starting), but that's not
too expensive.
Bug: webrtc:12614
Change-Id: I7b4447ccd88bd43d181e158f0d29b0770c8a3fd6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217522
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33926}
2021-05-05 12:36:52 +02:00
|
|
|
RTC_DCHECK(expiry_ == TimeMs::InfiniteFuture());
|
2021-03-30 22:54:41 +02:00
|
|
|
timeout_id_ = timeout_id;
|
2021-04-13 14:52:53 +02:00
|
|
|
expiry_ = get_time_() + duration_ms;
|
2021-03-30 22:54:41 +02:00
|
|
|
}
|
dcsctp: Handle starting timer from timer callback
This was caught in an integration test which had stricter assertions
than the FakeTimeout which is used in unit tests, so the first thing was
to add the same assertions to the FakeTimeout.
The issue is that when a Timer triggers, and if it's set to
automatically restart (possibly with an exponential backoff), the
`is_running_` field was set to true while the timer callback was called
to allow the client to know that the timer is in fact running, but the
timer was actually not started until the callback returned. Which made
sense, as the callback can with its return value override the duration,
which should affect the backoff algorithm.
The problem was when a timer was manually started within the callback.
As the Timer itself thought that it was already running, it first would
Stop() the underlying Timeout, then Start(). But calling Stop() on a
timeout that is not running is illegal, which set of assertions.
So the solution is to don't lie; Don't say that a timer is running when
it's not. Make sure that the timer is running when the timer callback is
triggered, which makes it consistent at all times. That may result in
unnecessary timeout invocations (stopping and starting), but that's not
too expensive.
Bug: webrtc:12614
Change-Id: I7b4447ccd88bd43d181e158f0d29b0770c8a3fd6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217522
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33926}
2021-05-05 12:36:52 +02:00
|
|
|
void Stop() override {
|
|
|
|
|
RTC_DCHECK(expiry_ != TimeMs::InfiniteFuture());
|
|
|
|
|
expiry_ = TimeMs::InfiniteFuture();
|
|
|
|
|
}
|
2021-03-30 22:54:41 +02:00
|
|
|
|
|
|
|
|
bool EvaluateHasExpired(TimeMs now) {
|
|
|
|
|
if (now >= expiry_) {
|
dcsctp: Handle starting timer from timer callback
This was caught in an integration test which had stricter assertions
than the FakeTimeout which is used in unit tests, so the first thing was
to add the same assertions to the FakeTimeout.
The issue is that when a Timer triggers, and if it's set to
automatically restart (possibly with an exponential backoff), the
`is_running_` field was set to true while the timer callback was called
to allow the client to know that the timer is in fact running, but the
timer was actually not started until the callback returned. Which made
sense, as the callback can with its return value override the duration,
which should affect the backoff algorithm.
The problem was when a timer was manually started within the callback.
As the Timer itself thought that it was already running, it first would
Stop() the underlying Timeout, then Start(). But calling Stop() on a
timeout that is not running is illegal, which set of assertions.
So the solution is to don't lie; Don't say that a timer is running when
it's not. Make sure that the timer is running when the timer callback is
triggered, which makes it consistent at all times. That may result in
unnecessary timeout invocations (stopping and starting), but that's not
too expensive.
Bug: webrtc:12614
Change-Id: I7b4447ccd88bd43d181e158f0d29b0770c8a3fd6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217522
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33926}
2021-05-05 12:36:52 +02:00
|
|
|
expiry_ = TimeMs::InfiniteFuture();
|
2021-03-30 22:54:41 +02:00
|
|
|
return true;
|
|
|
|
|
}
|
|
|
|
|
return false;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
TimeoutID timeout_id() const { return timeout_id_; }
|
|
|
|
|
|
|
|
|
|
private:
|
|
|
|
|
const std::function<TimeMs()> get_time_;
|
|
|
|
|
const std::function<void(FakeTimeout*)> on_delete_;
|
|
|
|
|
|
|
|
|
|
TimeoutID timeout_id_ = TimeoutID(0);
|
dcsctp: Handle starting timer from timer callback
This was caught in an integration test which had stricter assertions
than the FakeTimeout which is used in unit tests, so the first thing was
to add the same assertions to the FakeTimeout.
The issue is that when a Timer triggers, and if it's set to
automatically restart (possibly with an exponential backoff), the
`is_running_` field was set to true while the timer callback was called
to allow the client to know that the timer is in fact running, but the
timer was actually not started until the callback returned. Which made
sense, as the callback can with its return value override the duration,
which should affect the backoff algorithm.
The problem was when a timer was manually started within the callback.
As the Timer itself thought that it was already running, it first would
Stop() the underlying Timeout, then Start(). But calling Stop() on a
timeout that is not running is illegal, which set of assertions.
So the solution is to don't lie; Don't say that a timer is running when
it's not. Make sure that the timer is running when the timer callback is
triggered, which makes it consistent at all times. That may result in
unnecessary timeout invocations (stopping and starting), but that's not
too expensive.
Bug: webrtc:12614
Change-Id: I7b4447ccd88bd43d181e158f0d29b0770c8a3fd6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217522
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33926}
2021-05-05 12:36:52 +02:00
|
|
|
TimeMs expiry_ = TimeMs::InfiniteFuture();
|
2021-03-30 22:54:41 +02:00
|
|
|
};
|
|
|
|
|
|
|
|
|
|
class FakeTimeoutManager {
|
|
|
|
|
public:
|
|
|
|
|
// The `get_time` function must return the current time, relative to any
|
|
|
|
|
// epoch.
|
|
|
|
|
explicit FakeTimeoutManager(std::function<TimeMs()> get_time)
|
|
|
|
|
: get_time_(std::move(get_time)) {}
|
|
|
|
|
|
2022-01-26 18:38:13 +01:00
|
|
|
std::unique_ptr<FakeTimeout> CreateTimeout() {
|
2021-03-30 22:54:41 +02:00
|
|
|
auto timer = std::make_unique<FakeTimeout>(
|
|
|
|
|
get_time_, [this](FakeTimeout* timer) { timers_.erase(timer); });
|
|
|
|
|
timers_.insert(timer.get());
|
|
|
|
|
return timer;
|
|
|
|
|
}
|
2022-01-26 18:38:13 +01:00
|
|
|
std::unique_ptr<FakeTimeout> CreateTimeout(
|
|
|
|
|
webrtc::TaskQueueBase::DelayPrecision precision) {
|
|
|
|
|
// FakeTimeout does not support implement |precision|.
|
|
|
|
|
return CreateTimeout();
|
|
|
|
|
}
|
2021-03-30 22:54:41 +02:00
|
|
|
|
dcsctp: Expire timers just before triggering them
In real life, when a Timeout expires, the caller is supposed to call
DcSctpSocket::HandleTimeout directly, as the Timeout that just expired
is stopped (it just expired), but the Timer still believes it's running.
The system is not in a consistent state.
In tests, all timeouts were evaluated at the same time, which, if two
timeouts expired at the same time, would put them both as "not running",
and with their timers believing they were running. So if you would do
any operation on a timer whose timeout had just expired, the timeout
would assert saying that "you can't stop a stopped timeout" or similar.
This isn't relevant in non-test scenarios.
Solved by expiring timeouts one by one.
Bug: webrtc:12614
Change-Id: I79d006f4d3e96854d77cec3eb0080aa23b8569cb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217560
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33925}
2021-05-05 14:00:50 +02:00
|
|
|
// NOTE: This can't return a vector, as calling EvaluateHasExpired requires
|
|
|
|
|
// calling socket->HandleTimeout directly afterwards, as the owning Timer
|
|
|
|
|
// still believes it's running, and it needs to be updated to set
|
|
|
|
|
// Timer::is_running_ to false before you operate on the Timer or Timeout
|
|
|
|
|
// again.
|
|
|
|
|
absl::optional<TimeoutID> GetNextExpiredTimeout() {
|
2021-03-30 22:54:41 +02:00
|
|
|
TimeMs now = get_time_();
|
|
|
|
|
std::vector<TimeoutID> expired_timers;
|
|
|
|
|
for (auto& timer : timers_) {
|
|
|
|
|
if (timer->EvaluateHasExpired(now)) {
|
dcsctp: Expire timers just before triggering them
In real life, when a Timeout expires, the caller is supposed to call
DcSctpSocket::HandleTimeout directly, as the Timeout that just expired
is stopped (it just expired), but the Timer still believes it's running.
The system is not in a consistent state.
In tests, all timeouts were evaluated at the same time, which, if two
timeouts expired at the same time, would put them both as "not running",
and with their timers believing they were running. So if you would do
any operation on a timer whose timeout had just expired, the timeout
would assert saying that "you can't stop a stopped timeout" or similar.
This isn't relevant in non-test scenarios.
Solved by expiring timeouts one by one.
Bug: webrtc:12614
Change-Id: I79d006f4d3e96854d77cec3eb0080aa23b8569cb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217560
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33925}
2021-05-05 14:00:50 +02:00
|
|
|
return timer->timeout_id();
|
2021-03-30 22:54:41 +02:00
|
|
|
}
|
|
|
|
|
}
|
dcsctp: Expire timers just before triggering them
In real life, when a Timeout expires, the caller is supposed to call
DcSctpSocket::HandleTimeout directly, as the Timeout that just expired
is stopped (it just expired), but the Timer still believes it's running.
The system is not in a consistent state.
In tests, all timeouts were evaluated at the same time, which, if two
timeouts expired at the same time, would put them both as "not running",
and with their timers believing they were running. So if you would do
any operation on a timer whose timeout had just expired, the timeout
would assert saying that "you can't stop a stopped timeout" or similar.
This isn't relevant in non-test scenarios.
Solved by expiring timeouts one by one.
Bug: webrtc:12614
Change-Id: I79d006f4d3e96854d77cec3eb0080aa23b8569cb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217560
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33925}
2021-05-05 14:00:50 +02:00
|
|
|
return absl::nullopt;
|
2021-03-30 22:54:41 +02:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
private:
|
|
|
|
|
const std::function<TimeMs()> get_time_;
|
2021-08-18 15:22:42 +02:00
|
|
|
webrtc::flat_set<FakeTimeout*> timers_;
|
2021-03-30 22:54:41 +02:00
|
|
|
};
|
|
|
|
|
|
|
|
|
|
} // namespace dcsctp
|
|
|
|
|
|
|
|
|
|
#endif // NET_DCSCTP_TIMER_FAKE_TIMEOUT_H_
|