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

ldap_cron.php open lots of LDAP connections without closing it before the end #57

Open
brenard opened this issue Dec 19, 2016 · 8 comments

Comments

@brenard
Copy link

brenard commented Dec 19, 2016

Hello,

I'm using your great plugin to sync my LDAP users in Joomla. I have around 10 000 users in this LDAP directory and the ldap_cron.php is quit problematic in this context : for each users return by the first request listing all matching users in LDAP, it's open another connection (at least one) to LDAP directory without closing it before the end of the script. In my case, that means around 10 000 simultaneous connections to my LDAP directory.

After studying this problem, I see several methods to fix this problem :

  • I mean all shmanic components could share an unique connection to LDAP directory. For instance, the getInstance method of SHLdap class could return an unique object reference for the same $id and $authorised ?
  • Otherwise, in ldap_cron.php, all LDAP connections have to be closed after processing each user. This seems to be what was originally intended : in ldap_cron.php, in users loop, an unset($adapter); it's done before looping (but not in all exceptions cases) and the SHLdap object have a __destruct() method to handle LDAP connection closing. I'm not an expert of this special __destruct() method, so I'm not sure this the good way to be sure to close the connection in all case but It is definitely the sexier one :) Regarding to PHP doc, "The destructor method will be called as soon as there are no other references to a particular object, or in any order during the shutdown sequence.". So, that mean we still have $adapter reference after even after unset. FYI, If I comment SHLdapHelper::triggerEvent() and SHUserHelper::save() called and add unset called on $options and $instance, LDAP connections are successfully closed.

Thank you for any help !

@brenard
Copy link
Author

brenard commented Jan 19, 2017

Hi,

This problem is quite problematic for instances with large user base. Nobody have encountered this problem ?

Thank you

@conconnl
Copy link

I have issues with the LDAP_Cron, it looks like its related to the number of connections.
Still searching for a solution, but I'm not a programmer so i can't propose a fix in GIT.

@ShMaunder
Copy link
Owner

This sounds pretty nasty. Its been a while since I last looked at these extensions. I'm planning on spending my weekend restoring the project's automated build & testing so I can fix some issues.

@conconnl
Copy link

What we found out today is that we needed to increase the allowed open files on server level, because the connections were not closed.
So i hope you can solve some issues on the connection part, thanks for you service.

@ShMaunder
Copy link
Owner

ShMaunder commented Jan 20, 2017

Oh dear, thats a horrible workaround.

Brenard's suggestion around unsetting references to objects is most likely the solution so stuff can clean-up / GC. However I do remember this project having some nasty references being passed around so we didn't need to keep re-querying the LDAP server with multiple requests for every LDAP plug-in activated - so it might not be as easy as it sounds.

@amzen
Copy link

amzen commented Jan 21, 2017

Thank you Shaun to come back and take care with your excellent baby. I belong to the same team that Brenard, we are disturbed with this problem and it seems quite hard to make the right change. Your work on ldap with JMapMyLDAP is a reference in the Joomla community. We can help you as well as possible with tests and feedback. Kind regards, Hugues

@brenard
Copy link
Author

brenard commented Feb 6, 2017

@ShMaunder : Do you found some time to take a look on it ?

@conconnl
Copy link

conconnl commented Feb 8, 2017

@ShMaunder
I have some extra information for you.
We needed to change the linux /etc/security/limits.conf file where we needed to add the parameter
` * - nofile 2048
Which by default is 1024 and we now went above the 1000 users within one query.

I have changed the MaxConnections setting in Windows AD for LDAP but not sure if this was needed.
The default is 5000 and I changed this to 10000

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

4 participants