Skip to content
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

Memory leak while getting multiple connections with the same parameters [JDBC249] #298

Closed
firebird-issue-importer opened this issue Apr 20, 2012 · 9 comments

Comments

@firebird-issue-importer

Submitted by: @alexeykovyazin

Currently to generate key for hash table of FBConnectionProperties field "mapper" is used. This field is being lazy initialized during the first getMapper() call.
So after the first such call hash key became different from stored, equals() gives the false result and another copy of FBConnectionProperties is added.
Getting new connection with the same URL and properties makes hash table to grow infinitely.
There is few bytes leak per connection, but this became critical after 20-30 thousands of connects/disconnects.

public class DriverTest {

protected static final int CONNECTIONS_COUNT = 1000000;

@test
public void createManyConnectionsWithSameProperties() throws Exception {
new FBDriver();
final Runtime rt = Runtime.getRuntime();
for ( int i = 0; i < CONNECTIONS_COUNT; i++ ) {
final Connection connection = DriverManager.getConnection(
"jdbc:firebirdsql:localhost/3050:/Library/Frameworks/Firebird.framework/Versions/Current/Resources/examples/empbuild/employee.fdb",
"SYSDBA", "masterkey"
);
connection.close();
if ( i % 50 == 0 ) {
System.gc();
System.out.printf(""+i
+" free mem: %d b\n",
rt.freeMemory()
);
}
}
}
}

Commits: 9acadc0

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 20, 2012

Commented by: @mrotteveel

Where is FBConnectionProperties being used as a key to a Map / HashTable? I quickly scanned to code, but couldn't find it.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 20, 2012

Commented by: @alexeykovyazin

sorry for incomplete description

FBConnectionProperties is not used as a key, key is instance of FBManagedConnectionFactory mcf
method AbstractDriver.createDataSource

private FBDataSource createDataSource(FBManagedConnectionFactory mcf) throws ResourceException {
FBDataSource dataSource = null;
synchronized (mcfToDataSourceMap)
{
dataSource = (FBDataSource)mcfToDataSourceMap.get(mcf);

       if \(dataSource == null\) \{
           dataSource = \(FBDataSource\)mcf\.createConnectionFactory\(\);
           mcfToDataSourceMap\.put\(mcf, dataSource\);
       \}
   \}
   return dataSource;

}

and FBManagedConnectionFactory in the implementation of equals/hashCode uses FBConnectionProperties.equals/hashCode

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 21, 2012

Modified by: @mrotteveel

assignee: Roman Rokytskyy [ rrokytskyy ] => Mark Rotteveel [ avalanche1979 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 21, 2012

Commented by: @mrotteveel

It looks like removing mapper from hashCode and equals, and make sure the defaultIsolation is recorded in FBConnectionProperties if it is changed will fix this specific problem.
However I think the hashCode of FBConnectionProperties is inherently unstable: if one of the values recorded in the internal HashMap properties is changed or the value of type, database, tpbMapping or defaultTransactionIsolation is changed. This may require more thinking to get right.

Limiting the hashCode to database and type could be good enough and leave the rest to equals. I will need to research and test this a little bit longer.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 21, 2012

Commented by: @mrotteveel

Committed fix; I reduced the hashCode calculation to the most stable fields so it should remain stable. Also fixed a potential thread safety issue.

I also changed the storage of mcfToDataSourceMap to a WeakHashMap to prevent a potential different memory leak, but I am not 100% sure if that worked (limited testing did not reveal it getting garbage collected).

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 21, 2012

Commented by: @mrotteveel

Fixed for Jaybird-2.2 after beta-1

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 21, 2012

Modified by: @mrotteveel

Fix Version: Jaybird 2.2 [ 10053 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 21, 2012

Modified by: @mrotteveel

status: Open [ 1 ] => Resolved [ 5 ]

resolution: Fixed [ 1 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Aug 18, 2012

Modified by: @mrotteveel

status: Resolved [ 5 ] => Closed [ 6 ]

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

Successfully merging a pull request may close this issue.

None yet
2 participants