[sleep] Add a test for robustness with errant event engines (#30622)

* [sleep] Make this robust against poorly implemented event engines

* Automated change: Fix sanity tests

* Reland x2: Make GetDefaultEventEngine return a shared_ptr

* x

* [sleep] Add test for sleep robustness with bad event engines

* Automated change: Fix sanity tests

* fix

* fix

* Automated change: Fix sanity tests

* Update sleep_test.cc

* Update mock_event_engine.h

* Automated change: Fix sanity tests

Co-authored-by: ctiller <ctiller@users.noreply.github.com>
Co-authored-by: AJ Heller <hork@google.com>
pull/31225/head
Craig Tiller 2 years ago committed by GitHub
parent 0e8a6515df
commit 0057598fed
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 1
      build_autogenerated.yaml
  2. 12
      src/core/lib/promise/sleep.cc
  3. 12
      test/core/event_engine/BUILD
  4. 60
      test/core/event_engine/mock_event_engine.h
  5. 1
      test/core/promise/BUILD
  6. 40
      test/core/promise/sleep_test.cc

@ -9240,6 +9240,7 @@ targets:
build: test
language: c++
headers:
- test/core/event_engine/mock_event_engine.h
- test/core/promise/test_wakeup_schedulers.h
src:
- test/core/promise/sleep_test.cc

@ -19,7 +19,6 @@
#include <utility>
#include <grpc/event_engine/event_engine.h>
#include <grpc/support/log.h>
#include "src/core/lib/event_engine/default_event_engine.h" // IWYU pragma: keep
#include "src/core/lib/gprpp/time.h"
@ -54,12 +53,9 @@ Poll<absl::Status> Sleep::operator()() {
}
Sleep::ActiveClosure::ActiveClosure(Timestamp deadline)
: waker_(Activity::current()->MakeOwningWaker()) {
auto engine = GetContext<EventEngine>();
GPR_ASSERT(engine != nullptr &&
"An EventEngine context is required for Promise Sleep");
timer_handle_ = engine->RunAfter(deadline - Timestamp::Now(), this);
}
: waker_(Activity::current()->MakeOwningWaker()),
timer_handle_(GetContext<EventEngine>()->RunAfter(
deadline - Timestamp::Now(), this)) {}
void Sleep::ActiveClosure::Run() {
ApplicationCallbackExecCtx callback_exec_ctx;
@ -76,7 +72,7 @@ void Sleep::ActiveClosure::Cancel() {
// If we cancel correctly then we must own both refs still and can simply
// delete without unreffing twice, otherwise try unreffing since this may be
// the last owned ref.
if (GetContext<EventEngine>()->Cancel(timer_handle_) || Unref()) {
if (HasRun() || GetContext<EventEngine>()->Cancel(timer_handle_) || Unref()) {
delete this;
}
}

@ -159,3 +159,15 @@ grpc_cc_library(
"//:gpr",
],
)
grpc_cc_library(
name = "mock_event_engine",
hdrs = ["mock_event_engine.h"],
external_deps = [
"absl/functional:any_invocable",
"absl/status",
"absl/status:statusor",
"gtest",
],
deps = ["//:event_engine_base_hdrs"],
)

@ -0,0 +1,60 @@
// Copyright 2022 gRPC authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
#ifndef GRPC_TEST_EVENT_ENGINE_MOCK_EVENT_ENGINE_H
#define GRPC_TEST_EVENT_ENGINE_MOCK_EVENT_ENGINE_H
#include <chrono>
#include <memory>
#include "absl/functional/any_invocable.h"
#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "gmock/gmock.h"
#include <grpc/event_engine/endpoint_config.h>
#include <grpc/event_engine/event_engine.h>
#include <grpc/event_engine/memory_allocator.h>
namespace grpc_event_engine {
namespace experimental {
class MockEventEngine : public EventEngine {
public:
MOCK_METHOD(
absl::StatusOr<std::unique_ptr<Listener>>, CreateListener,
(Listener::AcceptCallback on_accept,
absl::AnyInvocable<void(absl::Status)> on_shutdown,
const EndpointConfig& config,
std::unique_ptr<MemoryAllocatorFactory> memory_allocator_factory));
MOCK_METHOD(ConnectionHandle, Connect,
(OnConnectCallback on_connect, const ResolvedAddress& addr,
const EndpointConfig& args, MemoryAllocator memory_allocator,
Duration timeout));
MOCK_METHOD(bool, CancelConnect, (ConnectionHandle handle));
MOCK_METHOD(bool, IsWorkerThread, ());
MOCK_METHOD(std::unique_ptr<DNSResolver>, GetDNSResolver,
(const DNSResolver::ResolverOptions& options));
MOCK_METHOD(void, Run, (Closure * closure));
MOCK_METHOD(void, Run, (absl::AnyInvocable<void()> closure));
MOCK_METHOD(TaskHandle, RunAfter, (Duration when, Closure* closure));
MOCK_METHOD(TaskHandle, RunAfter,
(Duration when, absl::AnyInvocable<void()> closure));
MOCK_METHOD(bool, Cancel, (TaskHandle handle));
};
} // namespace experimental
} // namespace grpc_event_engine
#endif

@ -378,6 +378,7 @@ grpc_cc_test(
"//:exec_ctx",
"//:grpc",
"//:sleep",
"//test/core/event_engine:mock_event_engine",
],
)

@ -20,6 +20,7 @@
#include <utility>
#include <vector>
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include <grpc/grpc.h>
@ -29,8 +30,20 @@
#include "src/core/lib/iomgr/exec_ctx.h"
#include "src/core/lib/promise/exec_ctx_wakeup_scheduler.h"
#include "src/core/lib/promise/race.h"
#include "test/core/event_engine/mock_event_engine.h"
#include "test/core/promise/test_wakeup_schedulers.h"
using grpc_event_engine::experimental::EventEngine;
using grpc_event_engine::experimental::GetDefaultEventEngine;
using grpc_event_engine::experimental::MockEventEngine;
using testing::_;
using testing::DoAll;
using testing::Matcher;
using testing::Mock;
using testing::Return;
using testing::SaveArg;
using testing::StrictMock;
namespace grpc_core {
namespace {
@ -52,6 +65,33 @@ TEST(Sleep, Zzzz) {
EXPECT_GE(Timestamp::Now(), done_time);
}
TEST(Sleep, OverlyEagerEventEngine) {
StrictMock<MockEventEngine> mock_event_engine;
ExecCtx exec_ctx;
bool done = false;
// Schedule a sleep for a very long time.
Timestamp done_time = Timestamp::Now() + Duration::Seconds(1e6);
EventEngine::Closure* wakeup = nullptr;
EXPECT_CALL(mock_event_engine, RunAfter(_, Matcher<EventEngine::Closure*>(_)))
.WillOnce(
DoAll(SaveArg<1>(&wakeup), Return(EventEngine::TaskHandle{42, 123})));
auto activity = MakeActivity(
Sleep(done_time), InlineWakeupScheduler(),
[&done](absl::Status r) {
EXPECT_EQ(r, absl::OkStatus());
done = true;
},
static_cast<EventEngine*>(&mock_event_engine));
Mock::VerifyAndClearExpectations(&mock_event_engine);
EXPECT_NE(wakeup, nullptr);
EXPECT_FALSE(done);
// Schedule the wakeup instantaneously - It won't have passed the scheduled
// time yet, but sleep should believe the event engine.
wakeup->Run();
EXPECT_TRUE(done);
}
TEST(Sleep, AlreadyDone) {
ExecCtx exec_ctx;
Notification done;

Loading…
Cancel
Save