You are viewing a plain text version of this content. The canonical link for it is here.
Posted to api@directory.apache.org by Shawn McKinney <sm...@apache.org> on 2022/07/13 19:11:00 UTC

Not closing search cursor

Hello,

Recently noticed the fortress code is not closing the search cursor. Take the GroupDAO.find[1] example (listed below).

Same pattern across the codebase despite clear warnings in the doc:

"Don’t forget to close the cursor, otherwise the associated data remains in memory forever (causing a memory leak)! Best practice is to use try-with-resources statement.” [2]

Obviously, we don’t want to be leaking.  What’s the recommendation here, should I change the code?


GroupDAO.find

```java
List<Group> find( Group group ) throws FinderException
{
...
SearchCursor searchResults;
try
{
...
searchResults = search( …);
        
while ( searchResults.next() )
{
groupList.add( unloadLdapEntry( searchResults.getEntry(), sequence++ ) );
}

…

catch ( CursorException e )
{
  // no cursor close here
}
catch ( LdapException e )
{
  // or here
}
finally
{
 closeAdminConnection( ld );
 // and worse? Not here either
}

return groupList;
}
``` 

—
Shawn

[1](https://github.com/apache/directory-fortress-core/blob/master/src/main/java/org/apache/directory/fortress/core/impl/GroupDAO.java)

[2](https://directory.apache.org/api/user-guide/2.3-searching.html)


---------------------------------------------------------------------
To unsubscribe, e-mail: api-unsubscribe@directory.apache.org
For additional commands, e-mail: api-help@directory.apache.org


Re: Not closing search cursor

Posted by Shawn McKinney <sm...@apache.org>.
> 
> On Jul 13, 2022, at 9:09 PM, Maxim Solodovnik <so...@gmail.com> wrote:
> 
> try (SearchCursor searchResults = search( …);) {
>    .....
> }
> 
> :)))

> On Jul 14, 2022, at 10:50 PM, Emmanuel Lécharny <el...@gmail.com> wrote:
> 
> 
> I have updated the doco to show how to use the try-with-resource.
> 
> Thanks !

Hey Maxim and Emmanuel, 

I’ll shoehorn it in with the existing connection resource, that also has to be closed.  Currently it uses the finally block, which isn’t a good thing (either).

Using the w/ resource way is definitely the way to go.

Thanks for the help.

—
Shawn 

---------------------------------------------------------------------
To unsubscribe, e-mail: api-unsubscribe@directory.apache.org
For additional commands, e-mail: api-help@directory.apache.org


Re: Not closing search cursor

Posted by Emmanuel Lécharny <el...@gmail.com>.
Hi Shawn,

I have updated the doco to show how to use the try-with-resource.

Thanks !

On 14/07/2022 04:09, Maxim Solodovnik wrote:
> try (SearchCursor searchResults = search( …);) {
>      .....
> }
> 
> :)))
> 
> On Thu, 14 Jul 2022 at 02:11, Shawn McKinney <sm...@apache.org> wrote:
>>
>> Hello,
>>
>> Recently noticed the fortress code is not closing the search cursor. Take the GroupDAO.find[1] example (listed below).
>>
>> Same pattern across the codebase despite clear warnings in the doc:
>>
>> "Don’t forget to close the cursor, otherwise the associated data remains in memory forever (causing a memory leak)! Best practice is to use try-with-resources statement.” [2]
>>
>> Obviously, we don’t want to be leaking.  What’s the recommendation here, should I change the code?
>>
>>
>> GroupDAO.find
>>
>> ```java
>> List<Group> find( Group group ) throws FinderException
>> {
>> ...
>> SearchCursor searchResults;
>> try
>> {
>> ...
>> searchResults = search( …);
>>
>> while ( searchResults.next() )
>> {
>> groupList.add( unloadLdapEntry( searchResults.getEntry(), sequence++ ) );
>> }
>>
>> …
>>
>> catch ( CursorException e )
>> {
>>    // no cursor close here
>> }
>> catch ( LdapException e )
>> {
>>    // or here
>> }
>> finally
>> {
>>   closeAdminConnection( ld );
>>   // and worse? Not here either
>> }
>>
>> return groupList;
>> }
>> ```
>>
>> —
>> Shawn
>>
>> [1](https://github.com/apache/directory-fortress-core/blob/master/src/main/java/org/apache/directory/fortress/core/impl/GroupDAO.java)
>>
>> [2](https://directory.apache.org/api/user-guide/2.3-searching.html)
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: api-unsubscribe@directory.apache.org
>> For additional commands, e-mail: api-help@directory.apache.org
>>
> 
> 

-- 
*Emmanuel Lécharny - CTO* 205 Promenade des Anglais – 06200 NICE
T. +33 (0)4 89 97 36 50
P. +33 (0)6 08 33 32 61
emmanuel.lecharny@busit.com https://www.busit.com/

---------------------------------------------------------------------
To unsubscribe, e-mail: api-unsubscribe@directory.apache.org
For additional commands, e-mail: api-help@directory.apache.org


Re: Not closing search cursor

Posted by Maxim Solodovnik <so...@gmail.com>.
try (SearchCursor searchResults = search( …);) {
    .....
}

:)))

On Thu, 14 Jul 2022 at 02:11, Shawn McKinney <sm...@apache.org> wrote:
>
> Hello,
>
> Recently noticed the fortress code is not closing the search cursor. Take the GroupDAO.find[1] example (listed below).
>
> Same pattern across the codebase despite clear warnings in the doc:
>
> "Don’t forget to close the cursor, otherwise the associated data remains in memory forever (causing a memory leak)! Best practice is to use try-with-resources statement.” [2]
>
> Obviously, we don’t want to be leaking.  What’s the recommendation here, should I change the code?
>
>
> GroupDAO.find
>
> ```java
> List<Group> find( Group group ) throws FinderException
> {
> ...
> SearchCursor searchResults;
> try
> {
> ...
> searchResults = search( …);
>
> while ( searchResults.next() )
> {
> groupList.add( unloadLdapEntry( searchResults.getEntry(), sequence++ ) );
> }
>
> …
>
> catch ( CursorException e )
> {
>   // no cursor close here
> }
> catch ( LdapException e )
> {
>   // or here
> }
> finally
> {
>  closeAdminConnection( ld );
>  // and worse? Not here either
> }
>
> return groupList;
> }
> ```
>
> —
> Shawn
>
> [1](https://github.com/apache/directory-fortress-core/blob/master/src/main/java/org/apache/directory/fortress/core/impl/GroupDAO.java)
>
> [2](https://directory.apache.org/api/user-guide/2.3-searching.html)
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: api-unsubscribe@directory.apache.org
> For additional commands, e-mail: api-help@directory.apache.org
>


-- 
Best regards,
Maxim

---------------------------------------------------------------------
To unsubscribe, e-mail: api-unsubscribe@directory.apache.org
For additional commands, e-mail: api-help@directory.apache.org