[Libevent-users] Double FD close

[Roger Morris <rmorris@xxxxxxxxxxxxxxxx>]

I'm looking at this change a695a72 from back in 2.1.6 (copied below), and I feel
like the change was perhaps made quickly in reaction to a coverity
warning without
really thinking it through.

In this code, a new connection is accepted from a listener. That new
connection (new_fd)
is then passed to the user's callback (cb), where ownership of that fd
is effectively turned
over to the user.  In this one case where the user happens to close
the listener from within
the callback, change a695a72 makes the assumption that the user didn't
take ownership
of new_fd, and didn't already or won't later close new_fd.

This is the only scenario where, after passing the accepted new_fd to
the callback, that
libevent closes new_fd.  Am I missing something?  Is this correct behavior?

--Roger Morris


$ git show -W   a695a72
commit a695a720cda892c270736d127333d73553842094
Author: Mark Ellzey <socket@xxxxxxxxx>
Date:   Mon Apr 27 22:43:04 2015 -0400

    Fix potential fd leak in listener_read_cb()

    As pointed out by harlan_ in #libevent after running a coverity swee
    If the listener is free'd, 'new_fd' is never closed.

diff --git a/listener.c b/listener.c
[...]

                UNLOCK(lev);
                cb(lev, new_fd, (struct sockaddr*)&ss, (int)socklen,
                    user_data);
                LOCK(lev);

                if (lev->refcnt == 1) {
                        int freed = listener_decref_and_unlock(lev);
                        EVUTIL_ASSERT(freed);
+
+                       evutil_closesocket(new_fd);
                        return;
                }
                --lev->refcnt;
***********************************************************************
To unsubscribe, send an e-mail to majordomo@xxxxxxxxxxxxx with
unsubscribe libevent-users    in the body.