JMapMyLDAP icon indicating copy to clipboard operation
JMapMyLDAP copied to clipboard

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

Open brenard opened this issue 8 years ago • 8 comments

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 avatar Dec 19 '16 10:12 brenard

Hi,

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

Thank you

brenard avatar Jan 19 '17 10:01 brenard

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.

conconnl avatar Jan 19 '17 15:01 conconnl

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.

ShMaunder avatar Jan 20 '17 22:01 ShMaunder

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.

conconnl avatar Jan 20 '17 23:01 conconnl

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.

ShMaunder avatar Jan 20 '17 23:01 ShMaunder

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

amzen avatar Jan 21 '17 11:01 amzen

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

brenard avatar Feb 06 '17 14:02 brenard

@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

conconnl avatar Feb 08 '17 13:02 conconnl