node-ldapjs icon indicating copy to clipboard operation
node-ldapjs copied to clipboard

client leak Half-closes sockets

Open atiertant opened this issue 5 years ago • 6 comments

in client with node v10.19.0, each time client.unbind() is called or on errors, the socket isn't destroyed only socket.end() is called and leak an Half-closes socket.

# lsof | grep "can't identify protocol"
node      25454     root   23u     sock                0,6        0t0  773832799 can't identify protocol
node      25454     root   24u     sock                0,6        0t0  773840655 can't identify protocol
node      25456     root   23u     sock                0,6        0t0  773831258 can't identify protocol
node      25456     root   24u     sock                0,6        0t0  773831259 can't identify protocol
node      25456     root   25u     sock                0,6        0t0  773831938 can't identify protocol
node      25456     root   26u     sock                0,6        0t0  773831939 can't identify protocol
node      25456     root   27u     sock                0,6        0t0  773832361 can't identify protocol
node      25456     root   28u     sock                0,6        0t0  773832362 can't identify protocol
node      25456     root   29u     sock                0,6        0t0  773832913 can't identify protocol
node      25456     root   30u     sock                0,6        0t0  773832914 can't identify protocol
node      25456     root   31u     sock                0,6        0t0  773834034 can't identify protocol

after some time, node can't open any file and return error EMFILE: too many open files, open

replace socket.end() by socket.destroy() in lines: https://github.com/ldapjs/node-ldapjs/blob/master/lib/client/client.js#L1096 https://github.com/ldapjs/node-ldapjs/blob/master/lib/client/client.js#L1163 https://github.com/ldapjs/node-ldapjs/blob/master/lib/client/client.js#L1177 https://github.com/ldapjs/node-ldapjs/blob/master/lib/client/client.js#L1454 solved the problem

atiertant avatar Apr 22 '20 10:04 atiertant

You have linked to code on the master branch. Are you seeing this issue on the next branch?

jsumners avatar Apr 22 '20 11:04 jsumners

@jsumners i didn't test this branch but as it's the same socket.end() this should be the same result accordind to the nodejs documentation https://nodejs.org/api/net.html#net_socket_end_data_encoding_callback vs https://nodejs.org/api/net.html#net_socket_destroy_error

atiertant avatar Apr 22 '20 13:04 atiertant

socket.end() is used so that the communication can complete.

jsumners avatar Apr 22 '20 14:04 jsumners

@jsumners could the socket be destroyed once this is done or after some timout ?

atiertant avatar Apr 22 '20 14:04 atiertant

Very likely. It would take some research into how the socket is being leaked so that it can be reaped correctly.

jsumners avatar Apr 22 '20 15:04 jsumners

We're also seeing this behavior. ldapjs-1.0.2 On node 12.13.1

Note I cannot reproduce this on my local MacOS environment. We only see it in our dockerized production environments.

We are connecting via secure (ldaps) to an Active Directory server.

Rich-Taylor-oss avatar May 11 '20 17:05 Rich-Taylor-oss

👋

On February 22, 2023, we released version 3 of this library. As a result, we are closing this issue/pull request.

Please see issue #839 for more information, including how to proceed if you feel this closure is in error.

jsumners avatar Feb 22 '23 19:02 jsumners