Hello, I pushed a PR which fixes the issue and Contains a test (which fails if the fix is not Present. Besides the uneeded double Indirection this code also leads to prematurely Dropped listeners and therefore loses change events.
Unfortunately a CI Acrion on Girhub failed with a unrelated sporadic problem and I don’t have permission to restart it or integrate the PR. Can somebody go the needful or let me know if I Need a dummy push. Gruß Bernd Gary D. Gregory wrote on 23. May 2024 20:00 (GMT +02:00): > 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 > > Gruß Bernd — https://bernd.eckenfels.net --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org