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

Eclipse JDT does not clean up opened ZipFiles #2

Open
asiekierka opened this issue Dec 30, 2018 · 11 comments
Open

Eclipse JDT does not clean up opened ZipFiles #2

asiekierka opened this issue Dec 30, 2018 · 11 comments
Labels
reported upstream This issue has been reported to the relevant upstream projects

Comments

@asiekierka
Copy link

See FabricMC/fabric-loom#45 for examination.

Not sure if upstream will be interested in fixing this, or if a patch should be provided by Mercury, or both - up to you.

@asiekierka
Copy link
Author

modmuss50 provided a simple patch: FabricMC/fabric-loom#45 (comment)

@stephan-gh
Copy link
Member

Thanks for investigating this issue!
May I suggest that you attempt to bring this up upstream? The environment in that method is an internal class in the JDT API and I don't get access to it from Mercury. So it is indeed the best way to fix this directly in JDT.

@asiekierka
Copy link
Author

We're already discussing bringing it up upstream. What I fear is that it will take a significant time to fix on their behalf, which is why I was suggesting to - if possible - add a .patch to the collection for the time being.

@stephan-gh
Copy link
Member

stephan-gh commented Jan 5, 2019

At the moment, the patches only create a relocated copy of the ImportRewrite related packages, not of the entire JDT JAR. The original one from Maven is still used for everything else.

When we start patching internal JDT classes for Mercury, we need to create a complete relocated copy of JDT, and have all users of Mercury use the relocated JDT packages. JDT is big with many transitive dependencies, so this is something I would like to avoid at any cost.

I believe it is easier if you use a temporary workaround on your side, e.g. one of the following:

  • Compile a fixed version of JDT and override the dependency in your runtime classpath
  • Run Mercury in a separate Java process
  • If System.gc() works reliably enough to convince the GC to clean up the files, then running that after Mercury might even be enough.

And let's hope that it does not take that long to fix it upstream.

@jamierocks
Copy link
Member

Is this something that was ever brought up with the Eclipse team, if so could you link us in - this is definitely something that needs addressing.

@asiekierka
Copy link
Author

I'm honestly not sure. Sorry.

@stephan-gh
Copy link
Member

This looks like the same issue: https://bugs.eclipse.org/bugs/show_bug.cgi?id=547631

@asiekierka
Copy link
Author

Seems like it, yes!

@jamierocks jamierocks added the reported upstream This issue has been reported to the relevant upstream projects label Nov 23, 2019
@stephan-gh
Copy link
Member

Does adding System.gc() at the end of

private void cleanup() {
this.sourceDir = null;
this.outputDir = null;
this.context.clear();
}

avoid this problem as an (ugly) workaround?

If anybody is willing to test if this fixes the problem on Windows, this would be an acceptable workaround imo. Probably doesn't hurt to encourage to garbage collector to clean up the mess JDT produced either.

@asiekierka
Copy link
Author

I think that's what we've done in Fabric before, and it generally worked, but I don't think it can be considered a guaranteed 100% workaround. @modmuss50?

@modmuss50
Copy link
Contributor

It seems to help out with our issues, but its not 100% fix.

Using the file leak detector it still shows that a lot of files are being leaked.

See this loom issue for what we found out: FabricMC/fabric-loom#45

jpenilla added a commit to jpenilla/Mercury that referenced this issue Nov 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
reported upstream This issue has been reported to the relevant upstream projects
Projects
None yet
Development

No branches or pull requests

4 participants