Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unbind clients in close() function #3

Open
ghost opened this issue Jun 15, 2012 · 4 comments
Open

Unbind clients in close() function #3

ghost opened this issue Jun 15, 2012 · 4 comments

Comments

@ghost
Copy link

ghost commented Jun 15, 2012

After invoking the close() function in lib/ldapauth.js, node will not terminate normally because _adminClient and _userClient are still bound. I suggest to rewrite the code in order to unbind both clients. Suggestion (not very good because it does not regard errors when unbinding _adminClient):

LdapAuth.prototype.close = function (callback) {
 var self = this;
  self._adminClient.unbind(function (err) {
    self._userClient.unbind(function (err2) {
       callback(err2) });})

By the way, I have not seen that _adminBound is set to true, therefore I think that in the current close() function, the _adminClient will never be unbound.

vesse added a commit to vesse/node-ldapauth-fork that referenced this issue Aug 22, 2013
Previously no client was unbound (see trentm#3), now
unbinding both without keeping track if they were actually bound as this
seems to be working (and also destroy-method in ldapjs' connection pool
does so).
@njlg
Copy link

njlg commented Jul 25, 2014

Looks like this issue is still a problem with the current version (2.2.4).

@wimvanleuven
Copy link

I confirm it is an issue in 2.2.4. However I see the code is alraedy adapted to adjust for this problem.

Anyone knows what the plans are for a new release? It the head code stable enough?

@wimvanleuven
Copy link

I'm switching to vesse's ldapauth-fork to work around these issues. It's a pity that this deviates for a bug that remains pending for so long ... or just a new release is not created.

@trentm
Copy link
Owner

trentm commented Sep 27, 2014

Yah, sorry guys. I'm currently seeing if vesse would like to take over node-ldapauth... whether using his ldapauth-fork name or ldapauth in npm.

vesse added a commit to vesse/node-ldapauth-fork that referenced this issue Feb 19, 2015
Previously no client was unbound (see trentm#3), now
unbinding both without keeping track if they were actually bound as this
seems to be working (and also destroy-method in ldapjs' connection pool
does so).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants