From: Razvan Becheriu Date: Mon, 17 Nov 2025 20:51:58 +0000 (+0200) Subject: [#4141] restored hasError X-Git-Tag: Kea-3.1.4~23 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c9b8c09e96fc6ee215ed846a3198940d47fee83a;p=thirdparty%2Fkea.git [#4141] restored hasError --- diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc index a407398f46..17d71e2154 100644 --- a/src/lib/dhcp/iface_mgr.cc +++ b/src/lib/dhcp/iface_mgr.cc @@ -1195,9 +1195,18 @@ Pkt4Ptr IfaceMgr::receive4Indirect(uint32_t timeout_sec, uint32_t timeout_usec / // Let's find out which external socket has the data SocketCallbackInfo ex_sock; bool found = false; + bool fd_error = false; { std::lock_guard lock(callbacks_mutex_); - for (const SocketCallbackInfo& s : callbacks_) { + for (SocketCallbackInfo& s : callbacks_) { + if (fd_event_handler_->hasError(s.socket_)) { + fd_error = true; + errno = 0; + if (fcntl(s.socket_, F_GETFD) < 0 && (errno == EBADF)) { + s.unusable_ = true; + } + break; + } if (!fd_event_handler_->readReady(s.socket_)) { continue; } @@ -1212,6 +1221,9 @@ Pkt4Ptr IfaceMgr::receive4Indirect(uint32_t timeout_sec, uint32_t timeout_usec / } } } + if (fd_error) { + isc_throw(SocketFDError, strerror(errno)); + } if (ex_sock.callback_) { // Calling the external socket's callback provides its service @@ -1306,9 +1318,18 @@ Pkt4Ptr IfaceMgr::receive4Direct(uint32_t timeout_sec, uint32_t timeout_usec /* // Let's find out which socket has the data SocketCallbackInfo ex_sock; bool found = false; + bool fd_error = false; { std::lock_guard lock(callbacks_mutex_); - for (const SocketCallbackInfo& s : callbacks_) { + for (SocketCallbackInfo& s : callbacks_) { + if (fd_event_handler_->hasError(s.socket_)) { + fd_error = true; + errno = 0; + if (fcntl(s.socket_, F_GETFD) < 0 && (errno == EBADF)) { + s.unusable_ = true; + } + break; + } if (!fd_event_handler_->readReady(s.socket_)) { continue; } @@ -1323,6 +1344,9 @@ Pkt4Ptr IfaceMgr::receive4Direct(uint32_t timeout_sec, uint32_t timeout_usec /* } } } + if (fd_error) { + isc_throw(SocketFDError, strerror(errno)); + } if (ex_sock.callback_) { // Calling the external socket's callback provides its service @@ -1339,6 +1363,9 @@ Pkt4Ptr IfaceMgr::receive4Direct(uint32_t timeout_sec, uint32_t timeout_usec /* IfacePtr recv_if; for (const IfacePtr& iface : ifaces_) { for (const SocketInfo& s : iface->getSockets()) { + if (fd_event_handler_->hasError(s.sockfd_)) { + isc_throw(SocketFDError, strerror(errno)); + } if (fd_event_handler_->readReady(s.sockfd_)) { candidate.reset(new SocketInfo(s)); break; @@ -1442,9 +1469,18 @@ IfaceMgr::receive6Direct(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ ) // Let's find out which socket has the data SocketCallbackInfo ex_sock; bool found = false; + bool fd_error = false; { std::lock_guard lock(callbacks_mutex_); - for (const SocketCallbackInfo& s : callbacks_) { + for (SocketCallbackInfo& s : callbacks_) { + if (fd_event_handler_->hasError(s.socket_)) { + fd_error = true; + errno = 0; + if (fcntl(s.socket_, F_GETFD) < 0 && (errno == EBADF)) { + s.unusable_ = true; + } + break; + } if (!fd_event_handler_->readReady(s.socket_)) { continue; } @@ -1459,6 +1495,9 @@ IfaceMgr::receive6Direct(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ ) } } } + if (fd_error) { + isc_throw(SocketFDError, strerror(errno)); + } if (ex_sock.callback_) { // Calling the external socket's callback provides its service @@ -1474,6 +1513,9 @@ IfaceMgr::receive6Direct(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ ) // @todo: fix iface starvation for (const IfacePtr& iface : ifaces_) { for (const SocketInfo& s : iface->getSockets()) { + if (fd_event_handler_->hasError(s.sockfd_)) { + isc_throw(SocketFDError, strerror(errno)); + } if (fd_event_handler_->readReady(s.sockfd_)) { candidate.reset(new SocketInfo(s)); break; @@ -1574,9 +1616,18 @@ IfaceMgr::receive6Indirect(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ // Let's find out which external socket has the data SocketCallbackInfo ex_sock; bool found = false; + bool fd_error = false; { std::lock_guard lock(callbacks_mutex_); - for (const SocketCallbackInfo& s : callbacks_) { + for (SocketCallbackInfo& s : callbacks_) { + if (fd_event_handler_->hasError(s.socket_)) { + fd_error = true; + errno = 0; + if (fcntl(s.socket_, F_GETFD) < 0 && (errno == EBADF)) { + s.unusable_ = true; + } + break; + } if (!fd_event_handler_->readReady(s.socket_)) { continue; } @@ -1591,6 +1642,9 @@ IfaceMgr::receive6Indirect(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ } } } + if (fd_error) { + isc_throw(SocketFDError, strerror(errno)); + } if (ex_sock.callback_) { // Calling the external socket's callback provides its service diff --git a/src/lib/dhcp/tests/iface_mgr_unittest.cc b/src/lib/dhcp/tests/iface_mgr_unittest.cc index d69d93b6b3..ebfb8364cb 100644 --- a/src/lib/dhcp/tests/iface_mgr_unittest.cc +++ b/src/lib/dhcp/tests/iface_mgr_unittest.cc @@ -663,7 +663,7 @@ public: // and (at least under Centos 7.5), this does not interrupt the // select. For now, we'll only test this for direct receive. if (!queue_enabled) { - EXPECT_THROW(ifacemgr->receive4(10), SocketReadError); + EXPECT_THROW(ifacemgr->receive4(10), SocketFDError); } // Verify write fails. diff --git a/src/lib/util/epoll_event_handler.cc b/src/lib/util/epoll_event_handler.cc index 45e06c790a..5bfd687123 100644 --- a/src/lib/util/epoll_event_handler.cc +++ b/src/lib/util/epoll_event_handler.cc @@ -102,7 +102,17 @@ bool EPollEventHandler::readReady(int fd) { if (map_.find(fd) == map_.end()) { return (false); } - return (map_[fd]->events & EPOLLIN); + return (map_[fd]->events & (EPOLLIN | EPOLLRDHUP | EPOLLHUP)); +} + +bool EPollEventHandler::hasError(int fd) { + if (errors_.count(fd)) { + return (true); + } + if (map_.find(fd) == map_.end()) { + return (false); + } + return (map_[fd]->events & EPOLLERR); } void EPollEventHandler::clear() { diff --git a/src/lib/util/epoll_event_handler.h b/src/lib/util/epoll_event_handler.h index e61b60547e..cca6fda429 100644 --- a/src/lib/util/epoll_event_handler.h +++ b/src/lib/util/epoll_event_handler.h @@ -51,6 +51,13 @@ public: /// @return True if file descriptor is ready for reading. bool readReady(int fd); + /// @brief Check if file descriptor has error. + /// + /// @param fd The file descriptor. + /// + /// @return True if file descriptor has error. + virtual bool hasError(int fd); + /// @brief Clear registered file descriptors. void clear(); diff --git a/src/lib/util/fd_event_handler.cc b/src/lib/util/fd_event_handler.cc index 9f9b778e09..bc8d0d4ecf 100644 --- a/src/lib/util/fd_event_handler.cc +++ b/src/lib/util/fd_event_handler.cc @@ -18,5 +18,9 @@ FDEventHandler::HandlerType FDEventHandler::type() { return (type_); } +bool FDEventHandler::hasError(int /* fd */) { + return (false); +} + } // end of namespace isc::util } // end of namespace isc diff --git a/src/lib/util/fd_event_handler.h b/src/lib/util/fd_event_handler.h index 8cd1b7e13f..4409f32832 100644 --- a/src/lib/util/fd_event_handler.h +++ b/src/lib/util/fd_event_handler.h @@ -56,6 +56,13 @@ public: /// @return True if file descriptor is ready for reading. virtual bool readReady(int fd) = 0; + /// @brief Check if file descriptor has error. + /// + /// @param fd The file descriptor. + /// + /// @return True if file descriptor has error. + virtual bool hasError(int fd); + /// @brief Clear registered file descriptors. virtual void clear() = 0; diff --git a/src/lib/util/kqueue_event_handler.cc b/src/lib/util/kqueue_event_handler.cc index dfdbe92d3c..8ede3492f8 100644 --- a/src/lib/util/kqueue_event_handler.cc +++ b/src/lib/util/kqueue_event_handler.cc @@ -112,6 +112,19 @@ bool KQueueEventHandler::readReady(int fd) { return (false); } +bool KQueueEventHandler::hasError(int fd) { + if (errors_.count(fd)) { + return (true); + } + auto range = map_.equal_range(fd); + for (auto it = range.first; it != range.second; ++it) { + if ((it->second->flags & EV_EOF) || (it->second->filter == EV_ERROR)) { + return (true); + } + } + return (false); +} + void KQueueEventHandler::clear() { data_.clear(); used_data_.clear(); diff --git a/src/lib/util/kqueue_event_handler.h b/src/lib/util/kqueue_event_handler.h index e5893476e9..41834f93d4 100644 --- a/src/lib/util/kqueue_event_handler.h +++ b/src/lib/util/kqueue_event_handler.h @@ -53,6 +53,13 @@ public: /// @return True if file descriptor is ready for reading. bool readReady(int fd); + /// @brief Check if file descriptor has error. + /// + /// @param fd The file descriptor. + /// + /// @return True if file descriptor has error. + virtual bool hasError(int fd); + /// @brief Clear registered file descriptors. void clear(); diff --git a/src/lib/util/poll_event_handler.cc b/src/lib/util/poll_event_handler.cc index fef1f2b6bb..61c0a44175 100644 --- a/src/lib/util/poll_event_handler.cc +++ b/src/lib/util/poll_event_handler.cc @@ -52,7 +52,15 @@ bool PollEventHandler::readReady(int fd) { if (map_.find(fd) == map_.end()) { return (false); } - return (map_[fd]->revents & POLLIN); + return (map_[fd]->revents & (POLLIN | POLLHUP)); +} + + +bool PollEventHandler::hasError(int fd) { + if (map_.find(fd) == map_.end()) { + return (false); + } + return (map_[fd]->revents & (POLLERR | POLLNVAL)); } void PollEventHandler::clear() { diff --git a/src/lib/util/poll_event_handler.h b/src/lib/util/poll_event_handler.h index 6bee3175ac..8b8108ce2c 100644 --- a/src/lib/util/poll_event_handler.h +++ b/src/lib/util/poll_event_handler.h @@ -51,6 +51,13 @@ public: /// @return True if file descriptor is ready for reading. bool readReady(int fd); + /// @brief Check if file descriptor has error. + /// + /// @param fd The file descriptor. + /// + /// @return True if file descriptor has error. + virtual bool hasError(int fd); + /// @brief Clear registered file descriptors. void clear(); diff --git a/src/lib/util/tests/fd_event_handler_unittests.h b/src/lib/util/tests/fd_event_handler_unittests.h index 531a4251a3..b5cc5927a6 100644 --- a/src/lib/util/tests/fd_event_handler_unittests.h +++ b/src/lib/util/tests/fd_event_handler_unittests.h @@ -67,26 +67,34 @@ TEST_F(FDEventHandlerTest, events) { EXPECT_NO_THROW(handler_->add(pipe_fd_[0])); EXPECT_FALSE(handler_->readReady(pipe_fd_[0])); + EXPECT_FALSE(handler_->hasError(pipe_fd_[0])); EXPECT_FALSE(handler_->readReady(pipe_fd_[1])); + EXPECT_FALSE(handler_->hasError(pipe_fd_[1])); EXPECT_EQ(0, handler_->waitEvent(0, 1000)); EXPECT_FALSE(handler_->readReady(pipe_fd_[0])); + EXPECT_FALSE(handler_->hasError(pipe_fd_[0])); EXPECT_FALSE(handler_->readReady(pipe_fd_[1])); + EXPECT_FALSE(handler_->hasError(pipe_fd_[1])); EXPECT_EQ(1, write(pipe_fd_[1], &MARKER, sizeof(MARKER))); EXPECT_EQ(1, handler_->waitEvent(0, 1000)); EXPECT_TRUE(handler_->readReady(pipe_fd_[0])); + EXPECT_FALSE(handler_->hasError(pipe_fd_[0])); EXPECT_FALSE(handler_->readReady(pipe_fd_[1])); + EXPECT_FALSE(handler_->hasError(pipe_fd_[1])); EXPECT_EQ(1, write(pipe_fd_[1], &MARKER, sizeof(MARKER))); EXPECT_EQ(1, handler_->waitEvent(0, 1000)); EXPECT_TRUE(handler_->readReady(pipe_fd_[0])); + EXPECT_FALSE(handler_->hasError(pipe_fd_[0])); EXPECT_FALSE(handler_->readReady(pipe_fd_[1])); + EXPECT_FALSE(handler_->hasError(pipe_fd_[1])); unsigned char data; @@ -95,14 +103,18 @@ TEST_F(FDEventHandlerTest, events) { EXPECT_EQ(1, handler_->waitEvent(0, 1000)); EXPECT_TRUE(handler_->readReady(pipe_fd_[0])); + EXPECT_FALSE(handler_->hasError(pipe_fd_[0])); EXPECT_FALSE(handler_->readReady(pipe_fd_[1])); + EXPECT_FALSE(handler_->hasError(pipe_fd_[1])); EXPECT_EQ(1, read(pipe_fd_[0], &data, sizeof(data))); EXPECT_EQ(0, handler_->waitEvent(0, 1000)); EXPECT_FALSE(handler_->readReady(pipe_fd_[0])); + EXPECT_FALSE(handler_->hasError(pipe_fd_[0])); EXPECT_FALSE(handler_->readReady(pipe_fd_[1])); + EXPECT_FALSE(handler_->hasError(pipe_fd_[1])); EXPECT_NO_THROW(handler_->clear()); @@ -148,6 +160,7 @@ TEST_F(FDEventHandlerTest, badFD) { EXPECT_EQ(1, handler_->waitEvent(0, 1000)); EXPECT_TRUE(handler_->readReady(fd)); + EXPECT_FALSE(handler_->hasError(fd)); close(fd); @@ -165,19 +178,217 @@ TEST_F(FDEventHandlerTest, badFD) { if (handler_->type() == FDEventHandler::TYPE_SELECT) { EXPECT_EQ(-1, handler_->waitEvent(0, 1000)); EXPECT_TRUE(handler_->readReady(fd)); + EXPECT_FALSE(handler_->hasError(fd)); EXPECT_EQ(EBADF, errno); } else if (handler_->type() == FDEventHandler::TYPE_POLL) { EXPECT_EQ(1, handler_->waitEvent(0, 1000)); EXPECT_FALSE(handler_->readReady(fd)); + EXPECT_TRUE(handler_->hasError(fd)); EXPECT_EQ(0, errno); } else { EXPECT_EQ(-1, handler_->waitEvent(0, 1000)); EXPECT_FALSE(handler_->readReady(fd)); + EXPECT_TRUE(handler_->hasError(fd)); EXPECT_EQ(EBADF, errno); } close(pipe_fd_[1]); pipe(pipe_fd_); + + EXPECT_NO_THROW(handler_->clear()); + errno = 0; + + { + EXPECT_NO_THROW(handler_->add(pipe_fd_[0])); + + std::thread tr([this]() { + usleep(500); + close(pipe_fd_[1]); + }); + + EXPECT_EQ(1, handler_->waitEvent(1, 0)); + + EXPECT_EQ(0, errno); + + EXPECT_TRUE(handler_->readReady(pipe_fd_[0])); + if (handler_->type() == FDEventHandler::TYPE_KQUEUE) { + EXPECT_TRUE(handler_->hasError(pipe_fd_[0])); + } else { + EXPECT_FALSE(handler_->hasError(pipe_fd_[0])); + } + + tr.join(); + + close(pipe_fd_[0]); + pipe(pipe_fd_); + } + + EXPECT_NO_THROW(handler_->clear()); + errno = 0; + + { + EXPECT_NO_THROW(handler_->add(pipe_fd_[0])); + + std::thread tr([this]() { + usleep(500); + close(pipe_fd_[0]); + }); + + if (handler_->type() == FDEventHandler::TYPE_SELECT) { +#if defined(OS_LINUX) + EXPECT_EQ(1, handler_->waitEvent(1, 0)); + + EXPECT_EQ(0, errno); + + EXPECT_TRUE(handler_->readReady(pipe_fd_[0])); + EXPECT_FALSE(handler_->hasError(pipe_fd_[0])); +#else + EXPECT_EQ(-1, handler_->waitEvent(1, 0)); + + EXPECT_EQ(EBADF, errno); + + EXPECT_TRUE(handler_->readReady(pipe_fd_[0])); + EXPECT_FALSE(handler_->hasError(pipe_fd_[0])); +#endif + } else if (handler_->type() == FDEventHandler::TYPE_POLL) { +#if defined(OS_LINUX) + EXPECT_EQ(1, handler_->waitEvent(1, 0)); + + EXPECT_EQ(0, errno); + + EXPECT_FALSE(handler_->readReady(pipe_fd_[0])); + EXPECT_TRUE(handler_->hasError(pipe_fd_[0])); +#else + EXPECT_EQ(0, handler_->waitEvent(1, 0)); + + EXPECT_EQ(0, errno); + + EXPECT_FALSE(handler_->readReady(pipe_fd_[0])); + EXPECT_FALSE(handler_->hasError(pipe_fd_[0])); +#endif + } else { + EXPECT_EQ(0, handler_->waitEvent(1, 0)); + + EXPECT_EQ(0, errno); + + EXPECT_FALSE(handler_->readReady(pipe_fd_[0])); + EXPECT_FALSE(handler_->hasError(pipe_fd_[0])); + } + + tr.join(); + + close(pipe_fd_[1]); + pipe(pipe_fd_); + } + + EXPECT_NO_THROW(handler_->clear()); + errno = 0; + + { + EXPECT_NO_THROW(handler_->add(pipe_fd_[1])); + + std::thread tr([this]() { + usleep(500); + close(pipe_fd_[1]); + }); + + if (handler_->type() == FDEventHandler::TYPE_SELECT) { +#if defined(OS_LINUX) + EXPECT_EQ(1, handler_->waitEvent(1, 0)); + + EXPECT_EQ(0, errno); + + EXPECT_TRUE(handler_->readReady(pipe_fd_[1])); + EXPECT_FALSE(handler_->hasError(pipe_fd_[1])); +#else + EXPECT_EQ(-1, handler_->waitEvent(1, 0)); + + EXPECT_EQ(EBADF, errno); + + EXPECT_TRUE(handler_->readReady(pipe_fd_[1])); + EXPECT_FALSE(handler_->hasError(pipe_fd_[1])); +#endif + } else if (handler_->type() == FDEventHandler::TYPE_POLL) { +#if defined(OS_LINUX) + EXPECT_EQ(1, handler_->waitEvent(1, 0)); + + EXPECT_EQ(0, errno); + + EXPECT_FALSE(handler_->readReady(pipe_fd_[1])); + EXPECT_TRUE(handler_->hasError(pipe_fd_[1])); +#else + EXPECT_EQ(0, handler_->waitEvent(1, 0)); + + EXPECT_EQ(0, errno); + + EXPECT_FALSE(handler_->readReady(pipe_fd_[1])); + EXPECT_FALSE(handler_->hasError(pipe_fd_[1])); +#endif + } else { + EXPECT_EQ(0, handler_->waitEvent(1, 0)); + + EXPECT_EQ(0, errno); + + EXPECT_FALSE(handler_->readReady(pipe_fd_[1])); + EXPECT_FALSE(handler_->hasError(pipe_fd_[1])); + } + + tr.join(); + + close(pipe_fd_[0]); + pipe(pipe_fd_); + } + + EXPECT_NO_THROW(handler_->clear()); + errno = 0; + + { + EXPECT_NO_THROW(handler_->add(pipe_fd_[1])); + + std::thread tr([this]() { + usleep(500); + close(pipe_fd_[0]); + }); + if (handler_->type() == FDEventHandler::TYPE_POLL) { + EXPECT_EQ(1, handler_->waitEvent(1, 0)); + + EXPECT_EQ(0, errno); + +#if defined(OS_LINUX) + EXPECT_FALSE(handler_->readReady(pipe_fd_[1])); + EXPECT_TRUE(handler_->hasError(pipe_fd_[1])); +#else + EXPECT_TRUE(handler_->readReady(pipe_fd_[1])); + EXPECT_FALSE(handler_->hasError(pipe_fd_[1])); +#endif + } else if (handler_->type() == FDEventHandler::TYPE_EPOLL) { + EXPECT_EQ(1, handler_->waitEvent(1, 0)); + + EXPECT_EQ(0, errno); + + EXPECT_FALSE(handler_->readReady(pipe_fd_[1])); + EXPECT_TRUE(handler_->hasError(pipe_fd_[1])); + } else if (handler_->type() == FDEventHandler::TYPE_KQUEUE) { + EXPECT_EQ(1, handler_->waitEvent(1, 0)); + + EXPECT_EQ(0, errno); + + EXPECT_TRUE(handler_->readReady(pipe_fd_[1])); + EXPECT_TRUE(handler_->hasError(pipe_fd_[1])); + } else { + EXPECT_EQ(1, handler_->waitEvent(1, 0)); + + EXPECT_EQ(0, errno); + + EXPECT_TRUE(handler_->readReady(pipe_fd_[1])); + EXPECT_FALSE(handler_->hasError(pipe_fd_[1])); + } + + tr.join(); + + close(pipe_fd_[1]); + pipe(pipe_fd_); + } } TEST_F(FDEventHandlerTest, hup) { @@ -185,6 +396,12 @@ TEST_F(FDEventHandlerTest, hup) { close(pipe_fd_[1]); EXPECT_EQ(1, handler_->waitEvent(0, 1000)); EXPECT_TRUE(handler_->readReady(pipe_fd_[0])); + if (handler_->type() == FDEventHandler::TYPE_KQUEUE) { + EXPECT_TRUE(handler_->hasError(pipe_fd_[0])); + } else { + EXPECT_FALSE(handler_->hasError(pipe_fd_[0])); + } + close(pipe_fd_[0]); pipe(pipe_fd_); }