Fix socket leak #8

Merged
merged 1 commit into from Mar 26, 2013

Conversation

Projects
None yet
2 participants
@hulkholden
Contributor

hulkholden commented Mar 25, 2013

After running for a while (and thrashing connections doing a live-update thing), connections began to fail like this:

[debug] accept() failed: 24
[debug] awake on incoming
[debug] accept() failed: 24
[debug] awake on incoming

$ lsof -p 10680
daedalus 10680 paulholden 8u IPv4 0x61822fb722019d87 0t0 TCP localhost:sunproxyadmin (LISTEN)
daedalus 10680 paulholden 9u IPv4 0x61822fb720940c8f 0t0 TCP localhost:sunproxyadmin->localhost:62430 (CLOSE_WAIT)
daedalus 10680 paulholden 10u IPv4 0x61822fb7208d3c8f 0t0 TCP localhost:sunproxyadmin->localhost:62431 (CLOSE_WAIT)
daedalus 10680 paulholden 11u IPv4 0x61822fb71aa657df 0t0 TCP localhost:sunproxyadmin->localhost:62432 (CLOSE_WAIT)
daedalus 10680 paulholden 12u IPv4 0x61822fb72200daff 0t0 TCP localhost:sunproxyadmin->localhost:62433 (CLOSE_WAIT)
...
daedalus 10680 paulholden 252u IPv4 0x61822fb71997364f 0t0 TCP localhost:sunproxyadmin->localhost:62675 (CLOSE_WAIT)
daedalus 10680 paulholden 253u IPv4 0x61822fb71997196f 0t0 TCP localhost:sunproxyadmin->localhost:62676 (CLOSE_WAIT)
daedalus 10680 paulholden 254u IPv4 0x61822fb71996fc8f 0t0 TCP localhost:sunproxyadmin->localhost:62677 (CLOSE_WAIT)
daedalus 10680 paulholden 255u IPv4 0x61822fb71998dd87 0t0 TCP localhost:sunproxyadmin->localhost:62678 (CLOSE_WAIT)

My fix just checks connection->socket is still valid rather than the WB_ALIVE flag, which fixes the issue above.

(WebbyServerShutdown tears down sockets without doing any checking - maybe that should check socket != WB_INVALID_SOCKET too?)

Sockets were being leaked - the WB_ALIVE flags is cleared in various …
…places, but without calling wb_close_socket. When the connection was finally torn down, wb_close_client would only close the socket when WB_ALIVE was still set. Seems more robust to check socket != WB_INVALID_SOCKET?

deplinenoise added a commit that referenced this pull request Mar 26, 2013

@deplinenoise deplinenoise merged commit 86347fc into deplinenoise:master Mar 26, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment