Hi,
I prepared a Pull request with the suggested changes. Maybe this is
helpful ;).
Thank you for checking the issue, especially cause it's close to
christmas and there are maybe other important tasks too ;).
Andreas
Am 21.12.24 um 14:24 schrieb Emmanuel Lécharny:
Hi,
I'll give it a try. Anyway, as I'm in the process to close a release
quite soon, it's important to get this issue fixed!
Thanks for your patience and your insight!
On 21/12/2024 09:59, Andreas Schufft wrote:
Hi,
i checked out the latest master and the behaviour is the same.
I played a little bit locally and used the following to fix this
behaviour:
Changed:
org.apache.directory.ldap.client.api.AbstractPoolableLdapConnectionFactory.PooledLdapConnectionFactory#wrap
to
private LdapConnection wrap( LdapConnection ldapConnection )
{
return ldapConnection; // only return the connection, do
not wrap
}
and in
org.apache.directory.ldap.client.api.LdapConnectionPool#getConnection
create the wrapper (last line in method):
public LdapConnection getConnection() throws LdapException
{
LdapConnection connection;
try
{
connection = super.borrowObject();
if ( LOG.isTraceEnabled() )
{
LOG.trace( I18n.msg( I18n.MSG_04163_BORROWED_CONNECTION, connection ) );
}
}
catch ( LdapException | RuntimeException e )
{
throw e;
}
catch ( Exception e )
{
// wrap in runtime, but this should NEVER happen per
published
// contract as it only throws what the makeObject throws
and our
// PoolableLdapConnectionFactory only throws LdapException
LOG.error( I18n.err( I18n.ERR_04107_UNEXPECTED_THROWN_EXCEPTION,
e.getMessage() ), e );
throw new RuntimeException( e );
}
// Wrap the underlying connection and make sure that the object
is returned to this pool instance
return new PooledLdapConnection(connection, this);
}
In my opinion this should be enough. The pooled objects are either of
type PooledObject<LdapNetworkConnection> or
PooledObject<MonitoredLdapConnection> (In case of a validating/
nonvalidating connection factory).
I tried 3 tests:
- pass a DefaultPoolableLdapConnectionFactory to the pool constructor
- pass a ValidatingPoolableLdapConnectionFactory to the pool constructor
- do not pass any connection factory to the pool constructor
All tests are passing.
Cleaner way should be a complete removal of the wrap method and the
removal of this call in the references:
org.apache.directory.ldap.client.api.AbstractPoolableLdapConnectionFactory.PooledLdapConnectionFactory#bindConnection
org.apache.directory.ldap.client.api.AbstractPoolableLdapConnectionFactory.PooledLdapConnectionFactory#configureConnection
org.apache.directory.ldap.client.api.AbstractPoolableLdapConnectionFactory.PooledLdapConnectionFactory#newLdapConnection
org.apache.directory.ldap.client.api.AbstractPoolableLdapConnectionFactory.PooledLdapConnectionFactory#newUnboundLdapConnection
Also I this the pool attribute could become removed from the factory .
I think it's dangerous to keep the pool here. I'm able to pass in the
ConnectionFactory as argument in the LdapConnectionPool constructor.
Creating a connection factory outside and pass this factory to
multiple pools will probably cause other hard to debug problems...
The shutdown of LDAP network connections will occur during the
destroyObject method pool.
Thank you for investigating the issue
Andreas
---------------------------------------------------------------------
To unsubscribe, e-mail: api-unsubscr...@directory.apache.org
For additional commands, e-mail: api-h...@directory.apache.org
