diff --git a/include/envoy/api/io_error.h b/include/envoy/api/io_error.h index 247e102a49d5..d671dea790e1 100644 --- a/include/envoy/api/io_error.h +++ b/include/envoy/api/io_error.h @@ -71,6 +71,12 @@ template struct IoCallResult { */ bool ok() const { return err_ == nullptr; } + /** + * This return code is frequent enough that we have a separate function to check. + * @return true if the system call failed because the socket would block. + */ + bool wouldBlock() const { return !ok() && err_->getErrorCode() == IoError::IoErrorCode::Again; } + // TODO(danzh): rename it to be more meaningful, i.e. return_value_. ReturnValue rc_; IoErrorPtr err_; diff --git a/include/envoy/event/file_event.h b/include/envoy/event/file_event.h index e66289cdb4f0..cfcb298b3080 100644 --- a/include/envoy/event/file_event.h +++ b/include/envoy/event/file_event.h @@ -18,15 +18,38 @@ struct FileReadyType { static const uint32_t Closed = 0x4; }; -enum class FileTriggerType { Level, Edge }; +enum class FileTriggerType { + // See @man 7 epoll(7) + // They are used on all platforms for DNS and TCP listeners. + Level, + // See @man 7 epoll(7) + // They are used on all platforms that support Edge triggering as the default trigger type. + Edge, + // These are synthetic edge events managed by Envoy. They are based on level events and when they + // are activated they are immediately disabled. This makes them behave like Edge events. Then it + // is is the responsibility of the consumer of the event to reactivate the event + // when the socket operation would block. + // + // Their main application in Envoy is for Win32 which does not support edge-triggered events. They + // should be used in Win32 instead of level events. They can only be used in platforms where + // `PlatformDefaultTriggerType` is `FileTriggerType::EmulatedEdge`. + EmulatedEdge +}; -static constexpr FileTriggerType PlatformDefaultTriggerType -#ifdef WIN32 - // Libevent only supports Level trigger on Windows. - {FileTriggerType::Level}; +// For POSIX developers to get the Windows behavior of file events +// you need to add the following definition: +// `FORCE_LEVEL_EVENTS` +// You can do this with bazel if you add the following build/test options +// `--copt="-DFORCE_LEVEL_EVENTS"` +constexpr FileTriggerType determinePlatformPreferredEventType() { +#if defined(WIN32) || defined(FORCE_LEVEL_EVENTS) + return FileTriggerType::EmulatedEdge; #else - {FileTriggerType::Edge}; + return FileTriggerType::Edge; #endif +} + +static constexpr FileTriggerType PlatformDefaultTriggerType = determinePlatformPreferredEventType(); /** * Callback invoked when a FileEvent is ready for reading or writing. @@ -53,6 +76,16 @@ class FileEvent { * registered events and fire callbacks when they are active. */ virtual void setEnabled(uint32_t events) PURE; + + /** + * Add a single event from the event registration mark. + */ + virtual void registerEventIfEmulatedEdge(uint32_t event) PURE; + + /** + * Remove a single event from the event registration mark. + */ + virtual void unregisterEventIfEmulatedEdge(uint32_t event) PURE; }; using FileEventPtr = std::unique_ptr; diff --git a/source/common/event/file_event_impl.cc b/source/common/event/file_event_impl.cc index 8dafc9abb675..ac99c7ad47e7 100644 --- a/source/common/event/file_event_impl.cc +++ b/source/common/event/file_event_impl.cc @@ -12,7 +12,7 @@ namespace Event { FileEventImpl::FileEventImpl(DispatcherImpl& dispatcher, os_fd_t fd, FileReadyCb cb, FileTriggerType trigger, uint32_t events) - : cb_(cb), fd_(fd), trigger_(trigger), + : cb_(cb), fd_(fd), trigger_(trigger), enabled_events_(events), activation_cb_(dispatcher.createSchedulableCallback([this]() { ASSERT(injected_activation_events_ != 0); mergeInjectedEventsAndRunCb(0); @@ -21,9 +21,13 @@ FileEventImpl::FileEventImpl(DispatcherImpl& dispatcher, os_fd_t fd, FileReadyCb // an OOM condition and just crash. RELEASE_ASSERT(SOCKET_VALID(fd), ""); #ifdef WIN32 - RELEASE_ASSERT(trigger_ == FileTriggerType::Level, - "libevent does not support edge triggers on Windows"); + ASSERT(trigger_ != FileTriggerType::Edge, "libevent does not support edge triggers on Windows"); #endif + if constexpr (PlatformDefaultTriggerType != FileTriggerType::EmulatedEdge) { + ASSERT(trigger_ != FileTriggerType::EmulatedEdge, + "Cannot use EmulatedEdge events if they are not the default platform type"); + } + assignEvents(events, &dispatcher.base()); event_add(&raw_event_, nullptr); } @@ -48,9 +52,10 @@ void FileEventImpl::activate(uint32_t events) { void FileEventImpl::assignEvents(uint32_t events, event_base* base) { ASSERT(base != nullptr); + enabled_events_ = events; event_assign( &raw_event_, base, fd_, - EV_PERSIST | (trigger_ == FileTriggerType::Level ? 0 : EV_ET) | + EV_PERSIST | (trigger_ == FileTriggerType::Edge ? EV_ET : 0) | (events & FileReadyType::Read ? EV_READ : 0) | (events & FileReadyType::Write ? EV_WRITE : 0) | (events & FileReadyType::Closed ? EV_CLOSED : 0), @@ -75,6 +80,16 @@ void FileEventImpl::assignEvents(uint32_t events, event_base* base) { this); } +void FileEventImpl::updateEvents(uint32_t events) { + if (events == enabled_events_) { + return; + } + auto* base = event_get_base(&raw_event_); + event_del(&raw_event_); + assignEvents(events, base); + event_add(&raw_event_, nullptr); +} + void FileEventImpl::setEnabled(uint32_t events) { if (injected_activation_events_ != 0) { // Clear pending events on updates to the fd event mask to avoid delivering events that are no @@ -84,19 +99,62 @@ void FileEventImpl::setEnabled(uint32_t events) { injected_activation_events_ = 0; activation_cb_->cancel(); } + updateEvents(events); +} - auto* base = event_get_base(&raw_event_); - event_del(&raw_event_); - assignEvents(events, base); - event_add(&raw_event_, nullptr); +void FileEventImpl::unregisterEventIfEmulatedEdge(uint32_t event) { + // This constexpr if allows the compiler to optimize away the function on POSIX + if constexpr (PlatformDefaultTriggerType == FileTriggerType::EmulatedEdge) { + ASSERT((event & (FileReadyType::Read | FileReadyType::Write)) == event); + if (trigger_ == FileTriggerType::EmulatedEdge) { + auto new_event_mask = enabled_events_ & ~event; + updateEvents(new_event_mask); + } + } +} + +void FileEventImpl::registerEventIfEmulatedEdge(uint32_t event) { + // This constexpr if allows the compiler to optimize away the function on POSIX + if constexpr (PlatformDefaultTriggerType == FileTriggerType::EmulatedEdge) { + ASSERT((event & (FileReadyType::Read | FileReadyType::Write)) == event); + if (trigger_ == FileTriggerType::EmulatedEdge) { + auto new_event_mask = enabled_events_ | event; + if (event & FileReadyType::Read && (enabled_events_ & FileReadyType::Closed)) { + // We never ask for both early close and read at the same time. + new_event_mask = new_event_mask & ~FileReadyType::Read; + } + updateEvents(new_event_mask); + } + } } void FileEventImpl::mergeInjectedEventsAndRunCb(uint32_t events) { if (injected_activation_events_ != 0) { + // TODO(antoniovicente) remove this adjustment to activation events once ConnectionImpl can + // handle Read and Close events delivered together. + if constexpr (PlatformDefaultTriggerType == FileTriggerType::EmulatedEdge) { + if (events & FileReadyType::Closed && injected_activation_events_ & FileReadyType::Read) { + // We never ask for both early close and read at the same time. If close is requested + // keep that instead. + injected_activation_events_ = injected_activation_events_ & ~FileReadyType::Read; + } + } + events |= injected_activation_events_; injected_activation_events_ = 0; activation_cb_->cancel(); } + + // TODO(davinci26): This can be optimized further in (w)epoll backends using the `EPOLLONESHOT` + // flag. With this flag `EPOLLIN`/`EPOLLOUT` are automatically disabled when the event is + // activated. + if constexpr (PlatformDefaultTriggerType == FileTriggerType::EmulatedEdge) { + if (trigger_ == FileTriggerType::EmulatedEdge) { + unregisterEventIfEmulatedEdge(events & + (Event::FileReadyType::Write | Event::FileReadyType::Read)); + } + } + cb_(events); } diff --git a/source/common/event/file_event_impl.h b/source/common/event/file_event_impl.h index d85969a4bb0a..dabfadd1b691 100644 --- a/source/common/event/file_event_impl.h +++ b/source/common/event/file_event_impl.h @@ -22,14 +22,19 @@ class FileEventImpl : public FileEvent, ImplBase { // Event::FileEvent void activate(uint32_t events) override; void setEnabled(uint32_t events) override; + void unregisterEventIfEmulatedEdge(uint32_t event) override; + void registerEventIfEmulatedEdge(uint32_t event) override; private: void assignEvents(uint32_t events, event_base* base); void mergeInjectedEventsAndRunCb(uint32_t events); + void updateEvents(uint32_t events); FileReadyCb cb_; os_fd_t fd_; FileTriggerType trigger_; + // Enabled events for this fd. + uint32_t enabled_events_; // Injected FileReadyType events that were scheduled by recent calls to activate() and are pending // delivery. diff --git a/source/common/event/libevent_scheduler.h b/source/common/event/libevent_scheduler.h index 398aa2634012..d9aeecd387ad 100644 --- a/source/common/event/libevent_scheduler.h +++ b/source/common/event/libevent_scheduler.h @@ -108,10 +108,9 @@ class LibeventScheduler : public Scheduler, public CallbackScheduler { static constexpr int flagsBasedOnEventType() { if constexpr (Event::PlatformDefaultTriggerType == FileTriggerType::Level) { - // On Windows, EVLOOP_NONBLOCK will cause the libevent event_base_loop to run forever. - // This is because libevent only supports level triggering on Windows, and so the write - // event callbacks will trigger every time through the loop. Adding EVLOOP_ONCE ensures the - // loop will run at most once + // With level events, EVLOOP_NONBLOCK will cause the libevent event_base_loop to run + // forever. This is because the write event callbacks will trigger every time through the + // loop. Adding EVLOOP_ONCE ensures the loop will run at most once return EVLOOP_NONBLOCK | EVLOOP_ONCE; } return EVLOOP_NONBLOCK; diff --git a/source/common/network/io_socket_handle_impl.cc b/source/common/network/io_socket_handle_impl.cc index fb1c2e120ec5..4090824a4ef1 100644 --- a/source/common/network/io_socket_handle_impl.cc +++ b/source/common/network/io_socket_handle_impl.cc @@ -84,8 +84,18 @@ Api::IoCallUint64Result IoSocketHandleImpl::readv(uint64_t max_length, Buffer::R num_bytes_to_read += slice_length; } ASSERT(num_bytes_to_read <= max_length); - return sysCallResultToIoCallResult(Api::OsSysCallsSingleton::get().readv( + auto result = sysCallResultToIoCallResult(Api::OsSysCallsSingleton::get().readv( fd_, iov.begin(), static_cast(num_slices_to_read))); + + // Emulated edge events need to registered if the socket operation did not complete + // because the socket would block. + if constexpr (Event::PlatformDefaultTriggerType == Event::FileTriggerType::EmulatedEdge) { + // Some tests try to read without initializing the file_event. + if (result.wouldBlock() && file_event_) { + file_event_->registerEventIfEmulatedEdge(Event::FileReadyType::Read); + } + } + return result; } Api::IoCallUint64Result IoSocketHandleImpl::read(Buffer::Instance& buffer, uint64_t max_length) { @@ -103,6 +113,15 @@ Api::IoCallUint64Result IoSocketHandleImpl::read(Buffer::Instance& buffer, uint6 bytes_to_commit -= slices[i].len_; } buffer.commit(slices, num_slices); + + // Emulated edge events need to registered if the socket operation did not complete + // because the socket would block. + if constexpr (Event::PlatformDefaultTriggerType == Event::FileTriggerType::EmulatedEdge) { + // Some tests try to read without initializing the file_event. + if (result.wouldBlock() && file_event_) { + file_event_->registerEventIfEmulatedEdge(Event::FileReadyType::Read); + } + } return result; } @@ -120,8 +139,18 @@ Api::IoCallUint64Result IoSocketHandleImpl::writev(const Buffer::RawSlice* slice if (num_slices_to_write == 0) { return Api::ioCallUint64ResultNoError(); } - return sysCallResultToIoCallResult( + auto result = sysCallResultToIoCallResult( Api::OsSysCallsSingleton::get().writev(fd_, iov.begin(), num_slices_to_write)); + + // Emulated edge events need to registered if the socket operation did not complete + // because the socket would block. + if constexpr (Event::PlatformDefaultTriggerType == Event::FileTriggerType::EmulatedEdge) { + // Some tests try to write without initializing the file_event. + if (result.wouldBlock() && file_event_) { + file_event_->registerEventIfEmulatedEdge(Event::FileReadyType::Write); + } + } + return result; } Api::IoCallUint64Result IoSocketHandleImpl::write(Buffer::Instance& buffer) { @@ -131,6 +160,15 @@ Api::IoCallUint64Result IoSocketHandleImpl::write(Buffer::Instance& buffer) { if (result.ok() && result.rc_ > 0) { buffer.drain(static_cast(result.rc_)); } + + // Emulated edge events need to registered if the socket operation did not complete + // because the socket would block. + if constexpr (Event::PlatformDefaultTriggerType == Event::FileTriggerType::EmulatedEdge) { + // Some tests try to read without initializing the file_event. + if (result.wouldBlock() && file_event_) { + file_event_->registerEventIfEmulatedEdge(Event::FileReadyType::Write); + } + } return result; } @@ -168,7 +206,15 @@ Api::IoCallUint64Result IoSocketHandleImpl::sendmsg(const Buffer::RawSlice* slic message.msg_control = nullptr; message.msg_controllen = 0; const Api::SysCallSizeResult result = os_syscalls.sendmsg(fd_, &message, flags); - return sysCallResultToIoCallResult(result); + auto io_result = sysCallResultToIoCallResult(result); + // Emulated edge events need to registered if the socket operation did not complete + // because the socket would block. + if constexpr (Event::PlatformDefaultTriggerType == Event::FileTriggerType::EmulatedEdge) { + if (io_result.wouldBlock() && file_event_) { + file_event_->registerEventIfEmulatedEdge(Event::FileReadyType::Write); + } + } + return io_result; } else { const size_t space_v6 = CMSG_SPACE(sizeof(in6_pktinfo)); const size_t space_v4 = CMSG_SPACE(sizeof(in_pktinfo)); @@ -210,7 +256,15 @@ Api::IoCallUint64Result IoSocketHandleImpl::sendmsg(const Buffer::RawSlice* slic *(reinterpret_cast(pktinfo->ipi6_addr.s6_addr)) = self_ip->ipv6()->address(); } const Api::SysCallSizeResult result = os_syscalls.sendmsg(fd_, &message, flags); - return sysCallResultToIoCallResult(result); + auto io_result = sysCallResultToIoCallResult(result); + // Emulated edge events need to registered if the socket operation did not complete + // because the socket would block. + if constexpr (Event::PlatformDefaultTriggerType == Event::FileTriggerType::EmulatedEdge) { + if (io_result.wouldBlock() && file_event_) { + file_event_->registerEventIfEmulatedEdge(Event::FileReadyType::Write); + } + } + return io_result; } } @@ -298,7 +352,15 @@ Api::IoCallUint64Result IoSocketHandleImpl::recvmsg(Buffer::RawSlice* slices, hdr.msg_controllen = cmsg_space_; const Api::SysCallSizeResult result = Api::OsSysCallsSingleton::get().recvmsg(fd_, &hdr, 0); if (result.rc_ < 0) { - return sysCallResultToIoCallResult(result); + auto io_result = sysCallResultToIoCallResult(result); + // Emulated edge events need to registered if the socket operation did not complete + // because the socket would block. + if constexpr (Event::PlatformDefaultTriggerType == Event::FileTriggerType::EmulatedEdge) { + if (io_result.wouldBlock() && file_event_) { + file_event_->registerEventIfEmulatedEdge(Event::FileReadyType::Read); + } + } + return io_result; } RELEASE_ASSERT((hdr.msg_flags & MSG_CTRUNC) == 0, @@ -381,7 +443,15 @@ Api::IoCallUint64Result IoSocketHandleImpl::recvmmsg(RawSliceArrays& slices, uin fd_, mmsg_hdr.data(), num_packets_per_mmsg_call, MSG_TRUNC | MSG_WAITFORONE, nullptr); if (result.rc_ <= 0) { - return sysCallResultToIoCallResult(result); + auto io_result = sysCallResultToIoCallResult(result); + // Emulated edge events need to registered if the socket operation did not complete + // because the socket would block. + if constexpr (Event::PlatformDefaultTriggerType == Event::FileTriggerType::EmulatedEdge) { + if (io_result.wouldBlock() && file_event_) { + file_event_->registerEventIfEmulatedEdge(Event::FileReadyType::Read); + } + } + return io_result; } int num_packets_read = result.rc_; @@ -435,7 +505,15 @@ Api::IoCallUint64Result IoSocketHandleImpl::recvmmsg(RawSliceArrays& slices, uin Api::IoCallUint64Result IoSocketHandleImpl::recv(void* buffer, size_t length, int flags) { const Api::SysCallSizeResult result = Api::OsSysCallsSingleton::get().recv(fd_, buffer, length, flags); - return sysCallResultToIoCallResult(result); + auto io_result = sysCallResultToIoCallResult(result); + // Emulated edge events need to registered if the socket operation did not complete + // because the socket would block. + if constexpr (Event::PlatformDefaultTriggerType == Event::FileTriggerType::EmulatedEdge) { + if (io_result.wouldBlock() && file_event_) { + file_event_->registerEventIfEmulatedEdge(Event::FileReadyType::Read); + } + } + return io_result; } bool IoSocketHandleImpl::supportsMmsg() const { diff --git a/source/extensions/transport_sockets/tls/ssl_socket.cc b/source/extensions/transport_sockets/tls/ssl_socket.cc index 50b2d27926e2..37a1f8547952 100644 --- a/source/extensions/transport_sockets/tls/ssl_socket.cc +++ b/source/extensions/transport_sockets/tls/ssl_socket.cc @@ -293,6 +293,17 @@ void SslSocket::shutdownSsl() { if (info_->state() != Ssl::SocketState::ShutdownSent && callbacks_->connection().state() != Network::Connection::State::Closed) { int rc = SSL_shutdown(rawSsl()); + if constexpr (Event::PlatformDefaultTriggerType == Event::FileTriggerType::EmulatedEdge) { + // Windows operate under `EmulatedEdge`. These are level events that are artificially + // made to behave like edge events. And if the rc is 0 then in that case we want read + // activation resumption. This code is protected with an `constexpr` if, to minimize the tax + // on POSIX systems that operate in Edge events. + if (rc == 0) { + // See https://www.openssl.org/docs/manmaster/man3/SSL_shutdown.html + // if return value is 0, Call SSL_read() to do a bidirectional shutdown. + callbacks_->setReadBufferReady(); + } + } ENVOY_CONN_LOG(debug, "SSL shutdown: rc={}", callbacks_->connection(), rc); drainErrorQueue(); info_->setState(Ssl::SocketState::ShutdownSent); diff --git a/source/extensions/transport_sockets/tls/ssl_socket.h b/source/extensions/transport_sockets/tls/ssl_socket.h index 4c5f38e0fb14..81b4712979fd 100644 --- a/source/extensions/transport_sockets/tls/ssl_socket.h +++ b/source/extensions/transport_sockets/tls/ssl_socket.h @@ -70,8 +70,6 @@ class SslSocket : public Network::TransportSocket, void onFailure() override; Network::TransportSocketCallbacks* transportSocketCallbacks() override { return callbacks_; } - SSL* rawSslForTest() const { return rawSsl(); } - protected: SSL* rawSsl() const { return info_->ssl_.get(); } diff --git a/source/server/hot_restarting_parent.cc b/source/server/hot_restarting_parent.cc index 874c19b11a5a..6898c0db9999 100644 --- a/source/server/hot_restarting_parent.cc +++ b/source/server/hot_restarting_parent.cc @@ -29,7 +29,7 @@ void HotRestartingParent::initialize(Event::Dispatcher& dispatcher, Server::Inst ASSERT(events == Event::FileReadyType::Read); onSocketEvent(); }, - Event::PlatformDefaultTriggerType, Event::FileReadyType::Read); + Event::FileTriggerType::Edge, Event::FileReadyType::Read); internal_ = std::make_unique(&server); } diff --git a/test/common/event/BUILD b/test/common/event/BUILD index 2eaba5a4f461..d5448366b593 100644 --- a/test/common/event/BUILD +++ b/test/common/event/BUILD @@ -11,7 +11,6 @@ envoy_package() envoy_cc_test( name = "dispatcher_impl_test", srcs = ["dispatcher_impl_test.cc"], - tags = ["fails_on_windows"], deps = [ "//source/common/api:api_lib", "//source/common/event:deferred_task", @@ -30,7 +29,6 @@ envoy_cc_test( envoy_cc_test( name = "file_event_impl_test", srcs = ["file_event_impl_test.cc"], - tags = ["fails_on_windows"], deps = [ "//include/envoy/event:file_event_interface", "//source/common/event:dispatcher_includes", diff --git a/test/common/event/dispatcher_impl_test.cc b/test/common/event/dispatcher_impl_test.cc index 092075137a4f..468c79bb62b7 100644 --- a/test/common/event/dispatcher_impl_test.cc +++ b/test/common/event/dispatcher_impl_test.cc @@ -1247,7 +1247,7 @@ TEST_F(DispatcherWithWatchdogTest, TouchBeforeTimer) { } TEST_F(DispatcherWithWatchdogTest, TouchBeforeFdEvent) { - os_fd_t fd = os_sys_calls_.socket(AF_INET6, SOCK_STREAM, 0).rc_; + os_fd_t fd = os_sys_calls_.socket(AF_INET6, SOCK_DGRAM, 0).rc_; ASSERT_TRUE(SOCKET_VALID(fd)); ReadyWatcher watcher; @@ -1258,7 +1258,7 @@ TEST_F(DispatcherWithWatchdogTest, TouchBeforeFdEvent) { file_event->activate(FileReadyType::Read); InSequence s; - EXPECT_CALL(*watchdog_, touch()); + EXPECT_CALL(*watchdog_, touch()).Times(2); EXPECT_CALL(watcher, ready()); dispatcher_->run(Dispatcher::RunType::NonBlock); } diff --git a/test/common/event/file_event_impl_test.cc b/test/common/event/file_event_impl_test.cc index e75e78598c3c..974c498dc905 100644 --- a/test/common/event/file_event_impl_test.cc +++ b/test/common/event/file_event_impl_test.cc @@ -106,7 +106,7 @@ TEST_P(FileEventImplActivateTest, Activate) { } TEST_P(FileEventImplActivateTest, ActivateChaining) { - os_fd_t fd = os_sys_calls_.socket(domain(), SOCK_STREAM, 0).rc_; + os_fd_t fd = os_sys_calls_.socket(domain(), SOCK_DGRAM, 0).rc_; ASSERT_TRUE(SOCKET_VALID(fd)); Api::ApiPtr api = Api::createApiForTest(); @@ -170,7 +170,7 @@ TEST_P(FileEventImplActivateTest, ActivateChaining) { } TEST_P(FileEventImplActivateTest, SetEnableCancelsActivate) { - os_fd_t fd = os_sys_calls_.socket(domain(), SOCK_STREAM, 0).rc_; + os_fd_t fd = os_sys_calls_.socket(domain(), SOCK_DGRAM, 0).rc_; ASSERT_TRUE(SOCKET_VALID(fd)); Api::ApiPtr api = Api::createApiForTest(); diff --git a/test/common/network/connection_impl_test.cc b/test/common/network/connection_impl_test.cc index aab097ee10b8..deadc876a03c 100644 --- a/test/common/network/connection_impl_test.cc +++ b/test/common/network/connection_impl_test.cc @@ -1098,6 +1098,12 @@ TEST_P(ConnectionImplTest, WriteWithWatermarks) { Invoke(client_write_buffer_, &MockWatermarkBuffer::baseMove))); NiceMock os_sys_calls; TestThreadsafeSingletonInjector os_calls(&os_sys_calls); + + EXPECT_CALL(os_sys_calls, readv(_, _, _)) + .WillRepeatedly(Invoke([&](os_fd_t, const iovec*, int) -> Api::SysCallSizeResult { + return {-1, SOCKET_ERROR_AGAIN}; + })); + EXPECT_CALL(os_sys_calls, writev(_, _, _)) .WillOnce(Invoke([&](os_fd_t, const iovec*, int) -> Api::SysCallSizeResult { dispatcher_->exit(); @@ -1188,6 +1194,7 @@ TEST_P(ConnectionImplTest, WatermarkFuzzing) { EXPECT_CALL(os_sys_calls, writev(_, _, _)) .WillOnce(Invoke([&](os_fd_t, const iovec*, int) -> Api::SysCallSizeResult { client_write_buffer_->drain(bytes_to_flush); + dispatcher_->exit(); return {-1, SOCKET_ERROR_AGAIN}; })) .WillRepeatedly(Invoke([&](os_fd_t, const iovec*, int) -> Api::SysCallSizeResult { @@ -1195,7 +1202,7 @@ TEST_P(ConnectionImplTest, WatermarkFuzzing) { })); client_connection_->write(buffer_to_write, false); - dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + dispatcher_->run(Event::Dispatcher::RunType::Block); } EXPECT_CALL(client_callbacks_, onBelowWriteBufferLowWatermark()).Times(AnyNumber()); @@ -1832,7 +1839,7 @@ class MockTransportConnectionImplTest : public testing::Test { return new Buffer::WatermarkBuffer(below_low, above_high, above_overflow); })); - file_event_ = new Event::MockFileEvent; + file_event_ = new NiceMock; EXPECT_CALL(dispatcher_, createFileEvent_(0, _, _, _)) .WillOnce(DoAll(SaveArg<1>(&file_ready_cb_), Return(file_event_))); transport_socket_ = new NiceMock; diff --git a/test/extensions/filters/listener/tls_inspector/tls_inspector_benchmark.cc b/test/extensions/filters/listener/tls_inspector/tls_inspector_benchmark.cc index 5bed094fc34c..00f1e37aa50c 100644 --- a/test/extensions/filters/listener/tls_inspector/tls_inspector_benchmark.cc +++ b/test/extensions/filters/listener/tls_inspector/tls_inspector_benchmark.cc @@ -41,6 +41,8 @@ class FastMockListenerFilterCallbacks : public Network::MockListenerFilterCallba class FastMockFileEvent : public Event::FileEvent { void activate(uint32_t) override {} void setEnabled(uint32_t) override {} + void unregisterEventIfEmulatedEdge(uint32_t) override {} + void registerEventIfEmulatedEdge(uint32_t) override {} }; class FastMockDispatcher : public Event::MockDispatcher { diff --git a/test/extensions/transport_sockets/tls/ssl_socket_test.cc b/test/extensions/transport_sockets/tls/ssl_socket_test.cc index 403a08b61c3a..774b57116d58 100644 --- a/test/extensions/transport_sockets/tls/ssl_socket_test.cc +++ b/test/extensions/transport_sockets/tls/ssl_socket_test.cc @@ -4724,6 +4724,7 @@ TEST_P(SslSocketTest, DownstreamNotReadySslSocket) { auto transport_socket = server_ssl_socket_factory.createTransportSocket(nullptr); EXPECT_EQ(EMPTY_STRING, transport_socket->protocol()); EXPECT_EQ(nullptr, transport_socket->ssl()); + EXPECT_EQ(true, transport_socket->canFlushClose()); Buffer::OwnedImpl buffer; Network::IoResult result = transport_socket->doRead(buffer); EXPECT_EQ(Network::PostIoAction::Close, result.action_); @@ -4759,6 +4760,7 @@ TEST_P(SslSocketTest, UpstreamNotReadySslSocket) { auto transport_socket = client_ssl_socket_factory.createTransportSocket(nullptr); EXPECT_EQ(EMPTY_STRING, transport_socket->protocol()); EXPECT_EQ(nullptr, transport_socket->ssl()); + EXPECT_EQ(true, transport_socket->canFlushClose()); Buffer::OwnedImpl buffer; Network::IoResult result = transport_socket->doRead(buffer); EXPECT_EQ(Network::PostIoAction::Close, result.action_); diff --git a/test/mocks/event/mocks.h b/test/mocks/event/mocks.h index 861f0a10e340..957f06222521 100644 --- a/test/mocks/event/mocks.h +++ b/test/mocks/event/mocks.h @@ -227,6 +227,8 @@ class MockFileEvent : public FileEvent { MOCK_METHOD(void, activate, (uint32_t events)); MOCK_METHOD(void, setEnabled, (uint32_t events)); + MOCK_METHOD(void, registerEventIfEmulatedEdge, (uint32_t event)); + MOCK_METHOD(void, unregisterEventIfEmulatedEdge, (uint32_t event)); }; } // namespace Event diff --git a/test/per_file_coverage.sh b/test/per_file_coverage.sh index 8db9d7472ee5..1b33d066f2e4 100755 --- a/test/per_file_coverage.sh +++ b/test/per_file_coverage.sh @@ -3,7 +3,8 @@ # directory:coverage_percent # for existing directories with low coverage. declare -a KNOWN_LOW_COVERAGE=( -"source/common/network:95.6" +"source/common/event:93.5" # Emulated edge events guards don't report LCOV +"source/common/network:95.1" "source/common/http/http3:50.0" "source/common/tracing:94.9" "source/common/protobuf:94.3" @@ -62,7 +63,7 @@ declare -a KNOWN_LOW_COVERAGE=( "source/extensions/tracers:96.0" "source/extensions/tracers/opencensus:91.2" "source/extensions/tracers/xray:94.0" -"source/extensions/transport_sockets:95.3" +"source/extensions/transport_sockets:95.1" "source/extensions/transport_sockets/tap:95.6" "source/extensions/transport_sockets/tls:94.2" "source/extensions/transport_sockets/tls/ocsp:95.3" diff --git a/test/test_common/simulated_time_system_test.cc b/test/test_common/simulated_time_system_test.cc index 4234142114af..e46284925deb 100644 --- a/test/test_common/simulated_time_system_test.cc +++ b/test/test_common/simulated_time_system_test.cc @@ -202,17 +202,17 @@ TEST_P(SimulatedTimeSystemTest, TimerOrderAndRescheduleTimer) { // is delayed since it is rescheduled with a non-zero delta. advanceMsAndLoop(5); if (activateMode() == ActivateMode::DelayActivateTimers) { -#ifdef WIN32 - // Force it to run again to pick up next iteration callbacks. - // The event loop runs for a single iteration in NonBlock mode on Windows as a hack to work - // around LEVEL trigger fd registrations constantly firing events and preventing the NonBlock - // event loop from ever reaching the no-fd event and no-expired timers termination condition. It - // is not possible to get consistent event loop behavior since the time system does not override - // the base scheduler's run behavior, and libevent does not provide a mode where it runs at most - // N iterations before breaking out of the loop for us to prefer over the single iteration mode - // used on Windows. - advanceMsAndLoop(0); -#endif + if constexpr (Event::PlatformDefaultTriggerType == FileTriggerType::Level) { + // Force it to run again to pick up next iteration callbacks. + // The event loop runs for a single iteration in NonBlock mode on Windows as a hack to work + // around LEVEL trigger fd registrations constantly firing events and preventing the NonBlock + // event loop from ever reaching the no-fd event and no-expired timers termination condition. + // It is not possible to get consistent event loop behavior since the time system does not + // override the base scheduler's run behavior, and libevent does not provide a mode where it + // runs at most N iterations before breaking out of the loop for us to prefer over the single + // iteration mode used on Windows. + advanceMsAndLoop(0); + } EXPECT_EQ("p013p4", output_); } else { EXPECT_EQ("p0134", output_); @@ -252,11 +252,11 @@ TEST_P(SimulatedTimeSystemTest, TimerOrderDisableAndRescheduleTimer) { // re-enabled with a non-zero timeout. advanceMsAndLoop(5); if (activateMode() == ActivateMode::DelayActivateTimers) { -#ifdef WIN32 - // The event loop runs for a single iteration in NonBlock mode on Windows. Force it to run again - // to pick up next iteration callbacks. - advanceMsAndLoop(0); -#endif + if constexpr (Event::PlatformDefaultTriggerType == FileTriggerType::Level) { + // The event loop runs for a single iteration in NonBlock mode on Windows. Force it to run + // again to pick up next iteration callbacks. + advanceMsAndLoop(0); + } EXPECT_THAT(output_, testing::AnyOf("p03p14", "p03p41")); } else { EXPECT_EQ("p0314", output_);