Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#273 Socket::select() should check socket descriptors...

closed
Net (141)
5
2012-09-14
2009-06-25
No

... before passing them to FD_ISSET.

Hi Rob,

From a quick look at the issue, I'd say it is a bug in Socket::select(). When you close a socket, the socket object's file descriptor for the socket is set to POCO_INVALID_SOCKET. This causes the FD_ISSET to crash. Proper fix would be to check the descriptor for POCO_INVALID_SOCKET before passing it to FD_ISSET in Socket::select(). The reason why it probably works with TCP sockets is that a TCP socket needs more time to close, as there's some communication involved, whereas UDP sockets are closed immediately.

Best regards,

Günter


Hi there,

I'm having a problem with a SIGSEGV in Socket::select() when closing a
DatagramSocket. The socket has a ReadableNotification and
ShutdownNotification registered with a SocketReactor.

What I think happens is that when I destroy the DatagramSocket, the reactor
is likely to be still in the select call. In the meantime, the socket is
removed. When the system select() returns, it crashes in the code below,
probably because it is accessing a dead socket.

for (SocketList::const_iterator it = readList.begin(); it !=

readList.end(); ++it)
{
if (FD_ISSET(it->sockfd(), &fdRead)) <-- crash here
readyReadList.push_back(*it);
}

Since I haven't seen this happen with my StreamSockets, I'm possibly
completely on the wrong track :)

Can anyone tell me how to properly close a DatagramSocket when it's used in
a reactor?

Cheers!

Rob

Discussion

  • wrobbie
    wrobbie
    2009-06-30

    I fixed it with the patch below (no idea how to attach files here..). The fix in r1184 didn't do the trick for me.

    --- poco-1.3.5/Net/src/Socket.cpp 2009-05-13 02:05:56.000000000 +0800
    +++ native/Net/src//Socket.cpp 2009-06-30 15:09:17.000000000 +0800
    @@ -94,23 +94,29 @@
    FD_ZERO(&fdRead);
    for (SocketList::const_iterator it = readList.begin(); it != readList.end(); ++it)
    {
    - if (int(it->sockfd()) > nfd)
    - nfd = int(it->sockfd());
    - FD_SET(it->sockfd(), &fdRead);
    + if (it->sockfd() != POCO_INVALID_SOCKET) {
    + if (int(it->sockfd()) > nfd)
    + nfd = int(it->sockfd());
    + FD_SET(it->sockfd(), &fdRead);
    + }
    }
    FD_ZERO(&fdWrite);
    for (SocketList::const_iterator it = writeList.begin(); it != writeList.end(); ++it)
    {
    - if (int(it->sockfd()) > nfd)
    - nfd = int(it->sockfd());
    - FD_SET(it->sockfd(), &fdWrite);
    + if (it->sockfd() != POCO_INVALID_SOCKET) {
    + if (int(it->sockfd()) > nfd)
    + nfd = int(it->sockfd());
    + FD_SET(it->sockfd(), &fdWrite);
    + }
    }
    FD_ZERO(&fdExcept);
    for (SocketList::const_iterator it = exceptList.begin(); it != exceptList.end(); ++it)
    {
    - if (int(it->sockfd()) > nfd)
    - nfd = int(it->sockfd());
    - FD_SET(it->sockfd(), &fdExcept);
    + if (it->sockfd() != POCO_INVALID_SOCKET) {
    + if (int(it->sockfd()) > nfd)
    + nfd = int(it->sockfd());
    + FD_SET(it->sockfd(), &fdExcept);
    + }
    }
    if (nfd == 0) return 0;
    Poco::Timespan remainingTime(timeout);
    @@ -138,21 +144,25 @@
    SocketList readyReadList;
    for (SocketList::const_iterator it = readList.begin(); it != readList.end(); ++it)
    {
    - if (FD_ISSET(it->sockfd(), &fdRead))
    +
    + if (it->sockfd() != POCO_INVALID_SOCKET &&
    + FD_ISSET(it->sockfd(), &fdRead))
    readyReadList.push_back(it);
    }
    std::swap(readList, readyReadList);
    SocketList readyWriteList;
    for (SocketList::const_iterator it = writeList.begin(); it != writeList.end(); ++it)
    {
    - if (FD_ISSET(it->sockfd(), &fdWrite))
    + if (it->sockfd() != POCO_INVALID_SOCKET &&
    + FD_ISSET(it->sockfd(), &fdWrite))
    readyWriteList.push_back(
    it);
    }
    std::swap(writeList, readyWriteList);
    SocketList readyExceptList;
    for (SocketList::const_iterator it = exceptList.begin(); it != exceptList.end(); ++it)
    {
    - if (FD_ISSET(it->sockfd(), &fdExcept))
    + if (it->sockfd() != POCO_INVALID_SOCKET &&
    + FD_ISSET(it->sockfd(), &fdExcept))
    readyExceptList.push_back(*it);
    }
    std::swap(exceptList, readyExceptList);

     
  • fixed in 1.3.6