]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#4141] restored hasError
authorRazvan Becheriu <razvan@isc.org>
Mon, 17 Nov 2025 20:51:58 +0000 (22:51 +0200)
committerRazvan Becheriu <razvan@isc.org>
Fri, 21 Nov 2025 13:02:43 +0000 (13:02 +0000)
src/lib/dhcp/iface_mgr.cc
src/lib/dhcp/tests/iface_mgr_unittest.cc
src/lib/util/epoll_event_handler.cc
src/lib/util/epoll_event_handler.h
src/lib/util/fd_event_handler.cc
src/lib/util/fd_event_handler.h
src/lib/util/kqueue_event_handler.cc
src/lib/util/kqueue_event_handler.h
src/lib/util/poll_event_handler.cc
src/lib/util/poll_event_handler.h
src/lib/util/tests/fd_event_handler_unittests.h

index a407398f46be01db253e8f713d4d0ac6aaf25a16..17d71e21543ba5114adeed7ad78ca1c6c198fbd0 100644 (file)
@@ -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<std::mutex> 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<std::mutex> 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<std::mutex> 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<std::mutex> 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
index d69d93b6b3b4adbdcb1acae758cef64981b06688..ebfb8364cb4a1b715f877d27a73da4cd76e84d85 100644 (file)
@@ -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.
index 45e06c790a83ab4e1d433a4918a520f69f58fdf6..5bfd687123df44f803b4a149da7e2e203040301d 100644 (file)
@@ -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() {
index e61b60547ea36a03ad39b1470ccf7d5478553f7d..cca6fda4296866180d22cf33af53fe964f3b84b1 100644 (file)
@@ -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();
 
index 9f9b778e0997f07db8f4bd265e758c046057f4f9..bc8d0d4ecf17b4aed2dd14c81b80a6e9d67dd272 100644 (file)
@@ -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
index 8cd1b7e13fa2d6e1b802df4aebbcdf0f7f185435..4409f32832aa8191f9da45f1d0b7d95d3eb1c731 100644 (file)
@@ -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;
 
index dfdbe92d3cce4807b0a4bac7cfb73bff2dc40c39..8ede3492f8d096f2ea0f643a7f8a0ea95d1107ff 100644 (file)
@@ -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();
index e5893476e927b344edf19bc1dee65ca3d7c07321..41834f93d40867846e18d24b1bf2b04f31e26ab6 100644 (file)
@@ -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();
 
index fef1f2b6bbac8345896431420d53d89ac2001e96..61c0a44175ceeb09932228369215325fdd1542e9 100644 (file)
@@ -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() {
index 6bee3175ac674f1a88bee1b4c4f74bbf06d144d2..8b8108ce2caea4e48e3f82cd3e05c42fbf1362f3 100644 (file)
@@ -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();
 
index 531a4251a364ab678610328d0ac66638ede12220..b5cc5927a6403cc2a949a61ec287274bca2a0b58 100644 (file)
@@ -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_);
 }