You are viewing a plain text version of this content. The canonical link for it is here.
Posted to api@directory.apache.org by Andreas Schufft <ap...@rmslash.com> on 2024/12/20 14:35:53 UTC
PooledLdapConnection / DIRAPI-398
Hi,
we encountered the same problem as described in ticket DIRAPI-398 in
older versions of the API (before 2.1.7). The connection pools filled up
because the close method on the underlying LdapConnection simply closed
the connection instead of returning the connection to the pool. We build
a workaround for us based on Proxy.newProxyInstance( , new
Class[]{LdapConnection.class}). This works fine for. Starting with the
latest release 2.1.7 we wanted to remove our own proxy and use the
pooling provided by the API but unfortunaly this doesn't work for us.
When an object becomes returned to the pool (by calling
releaseConnection the underlying pool throws an exception:
IllegalStateException: Returned object not currently part of this pool
I checked also the PooledLdapConnectionTest which contains a test
(closeReleasesToPool) but this test only ferifies that the
releaseConnection method is called on the pool. Unfortunaly I didn't
find any integration tests which would show the problem.
A simple test which is executed against a real LDAP shows the problem:
@Test
void closeReleasedToPoolValidating() throws IOException,
LdapException {
final LdapConnectionConfig config = new LdapConnectionConfig();
config.setLdapHost( "testldap" );
config.setLdapPort(389);
final PooledObjectFactory factory = new
DefaultPoolableLdapConnectionFactory(config);
LdapConnectionPool pool = new LdapConnectionPool(factory);
try
{
final LdapConnection connection = pool.getConnection();
Assertions.assertDoesNotThrow(() -> connection.close());
}
catch( final Exception e ) {
Assertions.fail(e);
}
}
This will fail with the following error:
org.opentest4j.AssertionFailedError: Unexpected exception thrown:
java.lang.RuntimeException: java.lang.IllegalStateException: Returned
object not currently part of this pool
at
org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:152)
at
org.junit.jupiter.api.AssertDoesNotThrow.createAssertionFailedError(AssertDoesNotThrow.java:84)
at
org.junit.jupiter.api.AssertDoesNotThrow.assertDoesNotThrow(AssertDoesNotThrow.java:53)
at
org.junit.jupiter.api.AssertDoesNotThrow.assertDoesNotThrow(AssertDoesNotThrow.java:36)
at
org.junit.jupiter.api.Assertions.assertDoesNotThrow(Assertions.java:3199)
at
org.apache.directory.ldap.client.api.PooledLdapConnectionTest.closeReleasedToPoolNonValidating(PooledLdapConnectionTest.java:84)
at java.lang.reflect.Method.invoke(Method.java:498)
at java.util.ArrayList.forEach(ArrayList.java:1259)
at java.util.ArrayList.forEach(ArrayList.java:1259)
Caused by: java.lang.RuntimeException: java.lang.IllegalStateException:
Returned object not currently part of this pool
at
org.apache.directory.ldap.client.api.LdapConnectionPool.releaseConnection(LdapConnectionPool.java:189)
at
org.apache.directory.ldap.client.api.PooledLdapConnection.close(PooledLdapConnection.java:49)
at
org.apache.directory.ldap.client.api.PooledLdapConnectionTest.lambda$closeReleasedToPoolNonValidating$1(PooledLdapConnectionTest.java:84)
at
org.junit.jupiter.api.AssertDoesNotThrow.assertDoesNotThrow(AssertDoesNotThrow.java:49)
... 6 more
Caused by: java.lang.IllegalStateException: Returned object not
currently part of this pool
at
org.apache.commons.pool2.impl.GenericObjectPool.returnObject(GenericObjectPool.java:1031)
at
org.apache.directory.ldap.client.api.LdapConnectionPool.releaseConnection(LdapConnectionPool.java:176)
... 9 more
When I debug the test I can see that the object within the Pool
GenericObjectPool.allObjects contains the value DefaultPooledObject with
a PooledLdapConnection as object. The PooledLdapConnection contains the
attribute connection of type LdapNetworkConnection.
So the Pool contains objects of type PooledLdapConnection, but the
PooledLdapConnection will try to release the underlying connection to
the pool and this fails.
The LdapConnectionPool#getConnection borrows an object from the pool
(type PooledLdapConnection), but the PooledLdapConnection#close method
tries to return the connection attribute of the PooledLdapConnection.
The behaviour is a little bit different whether you use the final
DefaultPoolableLdapConnectionFactory or a
ValidatingPoolableLdapConnectionFactory. The error is the same, but the
object in the pools are of a different type.
If you are using the DefaultPoolableLdapConnectionFactory the pool
consists of PooledLdapConnection objects (wrapping LdapNetworkConnection
objects). When you are using a ValidatingPoolableLdapConnectionFactory
the pool consists of MonitoringLdapConnection object (wrapping
PooledLdapConnection objects).
Our only solution to this problem is staying on version 2.1.6 with our
dynamic proxy object.
Thanks
Andreas
---------------------------------------------------------------------
To unsubscribe, e-mail: api-unsubscribe@directory.apache.org
For additional commands, e-mail: api-help@directory.apache.org
Re: PooledLdapConnection / DIRAPI-398
Posted by Andreas Schufft <ap...@rmslash.com>.
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-unsubscribe@directory.apache.org
For additional commands, e-mail: api-help@directory.apache.org
Re: PooledLdapConnection / DIRAPI-398
Posted by Emmanuel Lécharny <el...@gmail.com>.
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
>
>
>
>
--
*Emmanuel Lécharny* P. +33 (0)6 08 33 32 61
elecharny@apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: api-unsubscribe@directory.apache.org
For additional commands, e-mail: api-help@directory.apache.org
Re: PooledLdapConnection / DIRAPI-398
Posted by Andreas Schufft <ap...@rmslash.com>.
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-unsubscribe@directory.apache.org
For additional commands, e-mail: api-help@directory.apache.org
Re: PooledLdapConnection / DIRAPI-398
Posted by Emmanuel Lécharny <el...@gmail.com>.
Hi,
can you give the latest version (not released yet) a try? (You'll have
to checkout the code base and build it)
On 20/12/2024 15:35, Andreas Schufft wrote:
> Hi,
>
> we encountered the same problem as described in ticket DIRAPI-398 in
> older versions of the API (before 2.1.7). The connection pools filled up
> because the close method on the underlying LdapConnection simply closed
> the connection instead of returning the connection to the pool. We build
> a workaround for us based on Proxy.newProxyInstance( , new
> Class[]{LdapConnection.class}). This works fine for. Starting with the
> latest release 2.1.7 we wanted to remove our own proxy and use the
> pooling provided by the API but unfortunaly this doesn't work for us.
>
> When an object becomes returned to the pool (by calling
> releaseConnection the underlying pool throws an exception:
>
> IllegalStateException: Returned object not currently part of this pool
>
> I checked also the PooledLdapConnectionTest which contains a test
> (closeReleasesToPool) but this test only ferifies that the
> releaseConnection method is called on the pool. Unfortunaly I didn't
> find any integration tests which would show the problem.
>
> A simple test which is executed against a real LDAP shows the problem:
>
> @Test
> void closeReleasedToPoolValidating() throws IOException,
> LdapException {
> final LdapConnectionConfig config = new LdapConnectionConfig();
> config.setLdapHost( "testldap" );
> config.setLdapPort(389);
> final PooledObjectFactory factory = new
> DefaultPoolableLdapConnectionFactory(config);
> LdapConnectionPool pool = new LdapConnectionPool(factory);
> try
> {
> final LdapConnection connection = pool.getConnection();
> Assertions.assertDoesNotThrow(() -> connection.close());
> }
> catch( final Exception e ) {
> Assertions.fail(e);
> }
> }
>
> This will fail with the following error:
>
> org.opentest4j.AssertionFailedError: Unexpected exception thrown:
> java.lang.RuntimeException: java.lang.IllegalStateException: Returned
> object not currently part of this pool
>
> at
> org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:152)
> at
> org.junit.jupiter.api.AssertDoesNotThrow.createAssertionFailedError(AssertDoesNotThrow.java:84)
> at
> org.junit.jupiter.api.AssertDoesNotThrow.assertDoesNotThrow(AssertDoesNotThrow.java:53)
> at
> org.junit.jupiter.api.AssertDoesNotThrow.assertDoesNotThrow(AssertDoesNotThrow.java:36)
> at
> org.junit.jupiter.api.Assertions.assertDoesNotThrow(Assertions.java:3199)
> at
> org.apache.directory.ldap.client.api.PooledLdapConnectionTest.closeReleasedToPoolNonValidating(PooledLdapConnectionTest.java:84)
> at java.lang.reflect.Method.invoke(Method.java:498)
> at java.util.ArrayList.forEach(ArrayList.java:1259)
> at java.util.ArrayList.forEach(ArrayList.java:1259)
> Caused by: java.lang.RuntimeException: java.lang.IllegalStateException:
> Returned object not currently part of this pool
> at
> org.apache.directory.ldap.client.api.LdapConnectionPool.releaseConnection(LdapConnectionPool.java:189)
> at
> org.apache.directory.ldap.client.api.PooledLdapConnection.close(PooledLdapConnection.java:49)
> at
> org.apache.directory.ldap.client.api.PooledLdapConnectionTest.lambda$closeReleasedToPoolNonValidating$1(PooledLdapConnectionTest.java:84)
> at
> org.junit.jupiter.api.AssertDoesNotThrow.assertDoesNotThrow(AssertDoesNotThrow.java:49)
> ... 6 more
> Caused by: java.lang.IllegalStateException: Returned object not
> currently part of this pool
> at
> org.apache.commons.pool2.impl.GenericObjectPool.returnObject(GenericObjectPool.java:1031)
> at
> org.apache.directory.ldap.client.api.LdapConnectionPool.releaseConnection(LdapConnectionPool.java:176)
> ... 9 more
>
> When I debug the test I can see that the object within the Pool
>
> GenericObjectPool.allObjects contains the value DefaultPooledObject with
> a PooledLdapConnection as object. The PooledLdapConnection contains the
> attribute connection of type LdapNetworkConnection.
>
> So the Pool contains objects of type PooledLdapConnection, but the
> PooledLdapConnection will try to release the underlying connection to
> the pool and this fails.
>
>
> The LdapConnectionPool#getConnection borrows an object from the pool
> (type PooledLdapConnection), but the PooledLdapConnection#close method
> tries to return the connection attribute of the PooledLdapConnection.
>
> The behaviour is a little bit different whether you use the final
> DefaultPoolableLdapConnectionFactory or a
> ValidatingPoolableLdapConnectionFactory. The error is the same, but the
> object in the pools are of a different type.
>
> If you are using the DefaultPoolableLdapConnectionFactory the pool
> consists of PooledLdapConnection objects (wrapping LdapNetworkConnection
> objects). When you are using a ValidatingPoolableLdapConnectionFactory
> the pool consists of MonitoringLdapConnection object (wrapping
> PooledLdapConnection objects).
>
> Our only solution to this problem is staying on version 2.1.6 with our
> dynamic proxy object.
>
> Thanks
>
> Andreas
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: api-unsubscribe@directory.apache.org
> For additional commands, e-mail: api-help@directory.apache.org
>
--
*Emmanuel Lécharny* P. +33 (0)6 08 33 32 61
elecharny@apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: api-unsubscribe@directory.apache.org
For additional commands, e-mail: api-help@directory.apache.org