Skip to content

LDAP connection not closed due to DefaultDirContextValidator #489

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

Closed
antoine777 opened this issue Aug 2, 2018 · 7 comments · Fixed by #609
Closed

LDAP connection not closed due to DefaultDirContextValidator #489

antoine777 opened this issue Aug 2, 2018 · 7 comments · Fixed by #609
Assignees
Labels
Milestone

Comments

@antoine777
Copy link
Contributor

DefaultDirContextValidator class doesn't close NamingEnumeration searchResults.

As a consequence, the LDAP connection associated with the DirContext can't be closed when requested by the pool.

It's conform to the documentation : https://docs.oracle.com/javase/tutorial/jndi/ldap/close.html
"If the Context instance is sharing a connection with other Context and unterminated NamingEnumeration instances, the connection will not be closed until close() has been invoked on all such Context and NamingEnumeration instances"

DefaultDirContextValidator should close the NamingEnumeration searchResults in a finally block :

NamingEnumeration<SearchResult> = null;
...
try {
	searchResults = dirContext.search(this.base, this.filter, this.searchControls);
	...
} finally {
	if (searchResults != null) {
		try {
			searchResults.close();
		} catch (Exception e) {
		}
	}
}
@gaudryc
Copy link

gaudryc commented Mar 25, 2020

I confirm the need to close the NamingEnumeration object.
As long as the implementation of the "org.gaudryc.web.ldap.MyDirContextValidator.validateDirContext (DirContextType, DirContext)" method performs a search without closing the search result, my Tomcat 8.5 server reports, each time it its stops, existence of Threads blocked in reading on the sockets connected to the LDAP server :
24-Mar-2020 15:31:46.058 AVERTISSEMENT [localhost-startStop-2] org.apache.catalina.loader.WebappClassLoaderBase.clearReferencesThreads L'application web [test-ldap] semble avoir démarré un thread nommé [Thread-7] mais ne l'a pas arrêté, ce qui va probablement créer une fuite de mémoire; la trace du thread est:
java.net.SocketInputStream.socketRead0(Native Method)
java.net.SocketInputStream.socketRead(SocketInputStream.java:116)
java.net.SocketInputStream.read(SocketInputStream.java:171)
java.net.SocketInputStream.read(SocketInputStream.java:141)
java.io.BufferedInputStream.fill(BufferedInputStream.java:246)
java.io.BufferedInputStream.read1(BufferedInputStream.java:286)
java.io.BufferedInputStream.read(BufferedInputStream.java:345)
com.sun.jndi.ldap.Connection.run(Connection.java:846)
java.lang.Thread.run(Thread.java:748)

There is leak when the default validation process is enabled.
There is no leak when the default validation process is disabled.
There is no leak when the default validation process is enabled using a patched version of the validateDirContext method.

For your information, I'm using Java 11.0.6, Spring Ldap 2.3.2, Commons Pool 2.8, Spring 5.2.5 and Tomcat 8.5.53.

Thank you in advance for taking this problem into account.

Chris

@rwinch
Copy link
Member

rwinch commented Mar 27, 2020

Thanks for the report and follow up. Any chance one of you would be interested in submitting a PR for this?

@antoine777
Copy link
Contributor Author

PR done but Travis build failed.

@rwinch
Copy link
Member

rwinch commented Mar 30, 2020

@antoine777 Thanks for the PR. I think the issue is that we need to switch to openjdk as Travis no longer has oracle jdk 8. Would you mind sending a PR for this? This is what the change looked like for Spring Security spring-projects/spring-security@ca81421#diff-354f30a63fb0907d4ad57269548329e3

@antoine777
Copy link
Contributor Author

antoine777 commented Mar 31, 2020

@rwinch some other modifications were necessary to fix the build.

@rwinch
Copy link
Member

rwinch commented Apr 1, 2020

@antoine777 Thanks I commented on your PR

perry2of5 added a commit to perry2of5/spring-ldap that referenced this issue Jan 4, 2022
perry2of5 added a commit to perry2of5/spring-ldap that referenced this issue Jan 4, 2022
jzheaux pushed a commit that referenced this issue Jan 18, 2022
jzheaux added a commit that referenced this issue Jan 18, 2022
jzheaux pushed a commit that referenced this issue Jan 18, 2022
jzheaux added a commit that referenced this issue Jan 18, 2022
jzheaux added a commit that referenced this issue Jan 18, 2022
@jzheaux jzheaux self-assigned this Jan 18, 2022
@jzheaux jzheaux added in: core type: bug A general bug labels Jan 18, 2022
@jzheaux jzheaux added this to the 2.4.0-M2 milestone Jan 18, 2022
@michael-o
Copy link

Fantastic, 3,5 years later!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants