Hi Bernd, Thank you for researching this issue and presenting your findings.
In 2.9.0, we had (as you found): public static void installListener(final FileObject file, final FileListener listener) { final WeakRefFileListener weakListener = new WeakRefFileListener(file, listener); file.getFileSystem().addListener(file, new WeakRefFileListener(file, weakListener)); } Which already contains the "WeakRefFileListener in a WeakRefFileListener". Unless I am missing something, the change in git master now just in inlines the local variable. Are we saying that: (1) We should not have a "WeakRefFileListener in a WeakRefFileListener" (2) This problem already existed in 2.9.0. (3) This problem existed when the class was introduced in 2.2 (despite the 2.0 Javadoc since tag). Let me know how you see it. I can't say if your proposed change has any unintended side-effects; it should be accompanied with some kind of test, at the very least to avoid a regression. TY! Gary On 2024/05/23 17:08:00 Bernd Eckenfels wrote: > Hello, > > I am dealing with a heapdump of VFS where I see a lot of WeakRefFileListener > and all of them have a empty WeakRef to no listener. While I think I found the > reason for that and fixed it on a dependent project, it still does not clean > up correctly. I think the reason is that it does not store the WeakRefListener > directly, but it stores a WeakReflistener of it. This will then immediatelly > result in a unreferencedreferent - so it never works (it surely does fix the > leak :) > > Gary, while cleaning up lint errors in fba04f3e5 you made a change, but I > asume it was > only a mechanical replacement - or did you actually checkedif it is correct? > > > https://github.com/apache/commons-vfs/blob/dc9ad7677a020b2d4c571f7dcc858cdbae2bb538/commons-vfs2/src/main/java/org/apache/commons/vfs2/util/WeakRefFileListener.java#L41 > > Cleaned up code: > public static void installListener(final FileObject file, final FileListener > listener) { > file.getFileSystem().addListener(file, new WeakRefFileListener(file, new > WeakRefFileListener(file, listener))); > } > > initial version: > final WeakRefFileListener weakListener = new WeakRefFileListener(file, > listener); > file.getFileSystem().addListener(file, new WeakRefFileListener(file, > weakListener)); > > > but I think it should be only a single wrapper: > > public static void installListener(final FileObject file, final FileListener > listener) { > file.getFileSystem().addListener(file, new WeakRefFileListener(file, > listener)); > } > > There is a mention of VFS-143, but itintroduced the whole code with the > double indirection > and it does not explain why it is needed. > > What do you think, should we change it? > > I also wonder why no tests sees it (in factI try to add a test to reproduce > what I think > shows its not working). > > Gruss > Bernd > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org