-
Notifications
You must be signed in to change notification settings - Fork 51
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
java.lang.NoClassDefFoundError: org/eclipse/jgit/internal/JGitText #36
Comments
As far as I see, 5.13 does not even have that org.eclipse.jgit.internal.util.ShutdownHook class. This is supposed to be fixed in JGit 6.9.0 (issue #17, commit e6d83d6). However, I noticed it occurring in an EGit build, too. Probably some maven bundle uses a JGit 6.8.0 (the class was introduced in 6.8.0). The problem should vanish once JGit 6.9.0 gets pushed to maven (on simrel date 2024-03-13), and clients update their dependencies. |
However, I think Christoph has a point: this is conceptually broken. The change in commit e6d83d6 does not fix the problem: an exception already occurred, and most likely that itself was an ExecutionException wrapping another exception due to classes having been unloaded already. For instance LockFile registers LockFile::unlock to be run, but that itself probably is no longer valid, and if it is, it uses class FileUtils, which may also already be gone. In an OSGi environment these hooks should probably run when the org.eclipse.jgit bundle is stopped. A JVM shutdown hook is way too late. |
You thought about something like this ? |
Something similar: https://eclipse.gerrithub.io/c/eclipse-jgit/jgit/+/1177951 . Without dependencies on org.osgi.framework. |
Perhaps just saying: Stacktrace:
I would think the issue is the usage of JGitText inside the I would think hat this means is that all method of the |
So the same problem also exists with plexus classworlds. Maven closes the plexus container, which disposes the classworld realm, which closes the classloader. Note that the issue is not limited to JGitText. We could avoid that particular problem by pre-loading JGitText and keeping a reference around. But I fear it may occur on basically any class load attempt, and it's quite possible that the cleanup listeners may cause arbitrary classloading. Looks like JGit cannot use a shutdown hook at all. |
Pre-loading JGitText in ShutdownHook does get rid of this exception in a maven build. Verified with the reproducer from git-commit-id/git-commit-id-maven-plugin#712:
However: in the JGit build, I noticed this ShutdownHook being executed umpteen times at the end of the build (I put in a System.err.println to verify easily). JGit is a multi-module build, and multi-threaded to boot. Most likely this is not behaving the way it was intended... I'd be much happier if we could get rid of ShutdownHook (and the new OSGi CleanupService) and any use of Runtime.getRuntime().addShutdownHook() altogether. |
The text is now loaded but I still get Shutdown errors now, also I left some comments on the commit, I don't think that using a component is really appropriate here:
Maybe it would be good to revisit users of that API and check if there are alternatives. e.g. I looked where it is used, one case is the clone command, it seems to delete some files (recursively) if the clone was non done, but how can this happen, e.g. I shutdown the JVM while cloning? This does not looks appropriate as a ShutdownHook, it can literally take long depending on the size of the repository, and if I "kill" the JVM I likely can't expect anything "clean" afterwards. Then there is one that releases locks, also here I wonder if this is needed, what worse can happen is that the lookfile perists, will this be an issue? The other one is the config saver, what also looks suspicious, it say it can't use daemon threads because there is a risk of data corruption, also it claims it must use background threads because things tend to be slow but then it uses a fixed 100ms delay to wait for finish. Maybe one should better use some write and replace technique e.g. like:
|
Just for completeness this is the full stack trace now with
I think all |
If there are different classloaders (e.g. in maven each mojo / project has its own) there will be multiple instances of "singleton" |
Yes. As I wrote, pre-loading JGitText avoids this particular exception, but doesn't solve the other problems:
Stale lock files are a problem, but it looks as if a shutdown hook cannot solve this. |
I think using loggers is also dangerous, whyt I'm wondering is if not almost all (except the config one where I proposed an alternative), should simply use matching
calls instead of one global that is probably run if nothing needs to be done anymore.... As one might have perfomance in mind I'd like to reference the Oracle FAQ:
|
One point against using |
@iloveeclipse that why I think one should just add/remove the hook once it is no longer needed (at the place where they are used), e.g. the clone command has already try/finally and for the look files it is similar, once the lock is released it could also release the shutdown hook, currently this even references some lamdas that might even hold more references... This will make the |
I added ShutdownHook to fix concerns @tomaswolf raised when I added shutdown hooks for FileLock, compare PS 1 and 12: https://eclipse.gerrithub.io/c/eclipse-jgit/jgit/+/204213/1..12 This reduced occurrence of stale file locks in Gerrit when it's stopped gracefully. The shutdown hook for clone has the purpose to delete a partial clone if the cloning process is killed gracefully e.g. by hitting Ctrl-C when running it using jgit command line client. |
@msohn I think the listener approach is wrong (as explained by the oracle document at least hard to get right), I therefore would suggest to instead of adding/removing listeners, one is adding/removing a cleanup thread that way you make sure that your shutdown hook is only ever used when it is required and cleaned up afterwards. This should also gracefully solve the case for OSGi/Maven. |
For now solved by pre-loading JGitText. |
I still see this problem in GitHub Actions builds. |
In JGit 6.10.0, due June 12, 2024. |
Version
5.13.0 probably (the one used by tycho qualifier computer)
Operating System
Linux/Unix
Bug description
According to eclipse-pde/eclipse.pde#1020 (comment) , JGit seems to not properly unregister some services and that can cause further issue in systems that embed it, such as Tycho or Jenkins):
Actual behavior
When using Tycho, in some circumstances Maven build fails with
Expected behavior
No error.
Relevant log output
No response
Other information
Some analysis of the problem here: eclipse-pde/eclipse.pde#1020 (comment)
The text was updated successfully, but these errors were encountered: