taskadapter / redmine-java-api

Redmine Java API
Apache License 2.0
270 stars 163 forks source link

*manager instances from RedmineManager class cannot be cached and reused when RedmineManager's instance is up for garbage collection #222

Closed alexeyOnGitHub closed 9 years ago

alexeyOnGitHub commented 9 years ago

here is code in UserIntegrationTest:

@BeforeClass
public static void oneTimeSetup() {  
    RedmineManager mgr = IntegrationTestHelper.createRedmineManager();
    userManager = mgr.getUserManager();

you can see in this code that an instance of RedmineManager is created and mgr.getUserManager() result is saved to the instance var. but once this method finishesr, RedmineManager's "mgr" instance is up for garbage collection because its scope is within method. I checked in debugger that mgr's finalize() method is invoked and it calls shutdown() on the transport. which means as soon as "mgr" instance is garbage-collected, all its children manager instances will stop working. e.g. later calls to userManager (say, userManager.getCurrentUser()) fail because the transport is already shut down.

recent attempt to switch to Apache HttpClient 4.5.1 made this bug more apparent, but at the time we didn't have time to investigate this so I reverted HttpClient version upgrade. now the root cause is clear and it seems to be not related to HttpClient.

alexeyOnGitHub commented 9 years ago

I think we should delete finalize() from RedmineManager and document that clients must call shutdown() themselves when they are ready.

alexeyOnGitHub commented 9 years ago

fixed. this required breaking API changes. see the related commit.

bcmedeiros commented 9 years ago

UserManager shouldn't keep itself a reference to RedmineManager? Or to any other needed object? This bug is very weird..

alexeyOnGitHub commented 9 years ago

I updated the description. hopefully it is easier to read now.