pingidentity / ldapsdk

UnboundID LDAP SDK for Java
Other
334 stars 81 forks source link

Memory 'leak' in GSSAPIBindRequest #157

Closed griffjames100 closed 9 months ago

griffjames100 commented 10 months ago

Note: I've used the term 'leak', but its not technically a leak. However, the side effect is a constant upwards trend in JVM memory usage.

GSSAPIBindRequest uses deleteOnExit() which consumes a small amount of tracking memory each time it's called. This memory doesn't get freed until the JVM exit handling ensures all files tracked by deleteOnExit() are deleted.

Can you avoiding deleteOnExit(), and close the file explicitly instead?

Offending code is below. I can supply a heap dump if needed.

Thanks!

@NotNull() private static String getConfigFilePath( @NotNull final GSSAPIBindRequestProperties properties) throws LDAPException { try { final File f = File.createTempFile("GSSAPIBindRequest-JAAS-Config-", ".conf"); f.deleteOnExit();

griffjames100 commented 10 months ago

I think the correct fix would involve using the other LoginContext() constructor that accepts the Configuration object directly, instead of trying to use a temp file.

dirmgr commented 10 months ago

I'll see what I can do about this, but I may not be able to get to it until week after next. I have a couple of ideas about how this could potentially be addressed, but it will take some time to investigate.

griffjames100 commented 10 months ago

Thanks Neil, I know you don't accept code contributions... but if you want to discuss the local fix I've made to validate internally, let me know.

dirmgr commented 9 months ago

I'm sorry that it took substantially longer to get to this than I had originally hoped, but I just committed a change that should address the problem. I decided to go with a simpler change that just reuses an already generated JAAS config file if it would have otherwise generated a new file with the same contents. This means that if all of your binds use the same values for the properties that affect the contents of the JAAS config file, then it will only end up generating one config file and reusing it for all of those binds.

The properties that may be used in a generated JAAS config file are:

Other properties, like the authentication ID, authorization ID, password, QoP settings, etc., aren't used in the JAAS config file, so differences between them are ignored when determining whether it's possible to reuse an already generated configuration.

griffjames100 commented 9 months ago

Thanks Neil - much appreciated.

I made a similar short term fix in the client to specify a fixed Jaas file... I'll remove that and pull in your changes in the coming weeks.