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

java.lang.NoClassDefFoundError: org/eclipse/jgit/internal/JGitText #36

Closed
mickaelistria opened this issue Mar 8, 2024 · 19 comments
Closed
Milestone

Comments

@mickaelistria
Copy link
Contributor

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

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  02:09 min
[INFO] Finished at: 2024-01-30T08:06:00Z
[INFO] ------------------------------------------------------------------------
Error: Exception in thread "Thread-4" java.lang.NoClassDefFoundError: org/eclipse/jgit/internal/JGitText
	at org.eclipse.jgit.internal.util.ShutdownHook.cleanup(ShutdownHook.java:85)
	at java.base/java.lang.Thread.run(Thread.java:840)
Caused by: java.lang.ClassNotFoundException: org.eclipse.jgit.internal.JGitText
	... 2 more

Expected behavior

No error.

Relevant log output

No response

Other information

Some analysis of the problem here: eclipse-pde/eclipse.pde#1020 (comment)

@tomaswolf
Copy link
Contributor

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.

@tomaswolf
Copy link
Contributor

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.

@msohn
Copy link
Member

msohn commented Mar 8, 2024

You thought about something like this ?
https://eclipse.gerrithub.io/c/eclipse-jgit/jgit/+/1177774

@tomaswolf
Copy link
Contributor

Something similar: https://eclipse.gerrithub.io/c/eclipse-jgit/jgit/+/1177951 . Without dependencies on org.osgi.framework.

@TheSnoozer
Copy link

Perhaps just saying:
I just tested the latest released 6.9.0.202403050737-r and this java.lang.NoClassDefFoundError: org/eclipse/jgit/internal/JGitTextis still an issue.

Stacktrace:

ERROR] Cleanup during JVM shutdown failed
java.util.concurrent.ExecutionException: java.lang.NoClassDefFoundError
    at java.util.concurrent.ForkJoinTask.reportExecutionException (ForkJoinTask.java:581)
    at java.util.concurrent.ForkJoinTask.get (ForkJoinTask.java:1021)
    at org.eclipse.jgit.internal.util.ShutdownHook.cleanup (ShutdownHook.java:82)
    at java.lang.Thread.run (Thread.java:1583)
Caused by: java.lang.NoClassDefFoundError
    at jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance (DirectConstructorHandleAccessor.java:62)
    at java.lang.reflect.Constructor.newInstanceWithCaller (Constructor.java:502)
    at java.lang.reflect.Constructor.newInstance (Constructor.java:486)
    at java.util.concurrent.ForkJoinTask.getThrowableException (ForkJoinTask.java:542)
    at java.util.concurrent.ForkJoinTask.reportExecutionException (ForkJoinTask.java:580)
    at java.util.concurrent.ForkJoinTask.get (ForkJoinTask.java:1021)
    at org.eclipse.jgit.internal.util.ShutdownHook.cleanup (ShutdownHook.java:82)
    at java.lang.Thread.run (Thread.java:1583)
Caused by: java.lang.NoClassDefFoundError: org/eclipse/jgit/internal/JGitText
    at org.eclipse.jgit.internal.util.ShutdownHook.notify (ShutdownHook.java:97)
    at java.util.stream.ForEachOps$ForEachOp$OfRef.accept (ForEachOps.java:184)
    at java.util.concurrent.ConcurrentHashMap$KeySpliterator.forEachRemaining (ConcurrentHashMap.java:3573)
    at java.util.stream.AbstractPipeline.copyInto (AbstractPipeline.java:509)
    at java.util.stream.ForEachOps$ForEachTask.compute (ForEachOps.java:291)
    at java.util.concurrent.CountedCompleter.exec (CountedCompleter.java:754)
    at java.util.concurrent.ForkJoinTask.doExec (ForkJoinTask.java:387)
    at java.util.concurrent.ForkJoinTask.invoke (ForkJoinTask.java:667)
    at java.util.stream.ForEachOps$ForEachOp.evaluateParallel (ForEachOps.java:160)
    at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateParallel (ForEachOps.java:174)
    at java.util.stream.AbstractPipeline.evaluate (AbstractPipeline.java:233)
    at java.util.stream.ReferencePipeline.forEach (ReferencePipeline.java:596)
    at java.util.stream.ReferencePipeline$Head.forEach (ReferencePipeline.java:765)
    at org.eclipse.jgit.internal.util.ShutdownHook.doCleanup (ShutdownHook.java:93)
    at org.eclipse.jgit.internal.util.ShutdownHook.lambda$1 (ShutdownHook.java:80)
    at java.util.concurrent.ForkJoinTask$AdaptedCallable.exec (ForkJoinTask.java:1456)
    at java.util.concurrent.ForkJoinTask.doExec (ForkJoinTask.java:387)
    at java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec (ForkJoinPool.java:1312)
    at java.util.concurrent.ForkJoinPool.scan (ForkJoinPool.java:1843)
    at java.util.concurrent.ForkJoinPool.runWorker (ForkJoinPool.java:1808)
    at java.util.concurrent.ForkJoinWorkerThread.run (ForkJoinWorkerThread.java:188)
Caused by: java.lang.ClassNotFoundException: org.eclipse.jgit.internal.JGitText
    at org.codehaus.plexus.classworlds.strategy.SelfFirstStrategy.loadClass (SelfFirstStrategy.java:50)
    at org.codehaus.plexus.classworlds.realm.ClassRealm.unsynchronizedLoadClass (ClassRealm.java:271)
    at org.codehaus.plexus.classworlds.realm.ClassRealm.loadClass (ClassRealm.java:247)
    at org.codehaus.plexus.classworlds.realm.ClassRealm.loadClass (ClassRealm.java:239)
    at org.eclipse.jgit.internal.util.ShutdownHook.notify (ShutdownHook.java:97)
    at java.util.stream.ForEachOps$ForEachOp$OfRef.accept (ForEachOps.java:184)
    at java.util.concurrent.ConcurrentHashMap$KeySpliterator.forEachRemaining (ConcurrentHashMap.java:3573)
    at java.util.stream.AbstractPipeline.copyInto (AbstractPipeline.java:509)
    at java.util.stream.ForEachOps$ForEachTask.compute (ForEachOps.java:291)
    at java.util.concurrent.CountedCompleter.exec (CountedCompleter.java:754)
    at java.util.concurrent.ForkJoinTask.doExec (ForkJoinTask.java:387)
    at java.util.concurrent.ForkJoinTask.invoke (ForkJoinTask.java:667)
    at java.util.stream.ForEachOps$ForEachOp.evaluateParallel (ForEachOps.java:160)
    at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateParallel (ForEachOps.java:174)
    at java.util.stream.AbstractPipeline.evaluate (AbstractPipeline.java:233)
    at java.util.stream.ReferencePipeline.forEach (ReferencePipeline.java:596)
    at java.util.stream.ReferencePipeline$Head.forEach (ReferencePipeline.java:765)
    at org.eclipse.jgit.internal.util.ShutdownHook.doCleanup (ShutdownHook.java:93)
    at org.eclipse.jgit.internal.util.ShutdownHook.lambda$1 (ShutdownHook.java:80)
    at java.util.concurrent.ForkJoinTask$AdaptedCallable.exec (ForkJoinTask.java:1456)
    at java.util.concurrent.ForkJoinTask.doExec (ForkJoinTask.java:387)
    at java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec (ForkJoinPool.java:1312)
    at java.util.concurrent.ForkJoinPool.scan (ForkJoinPool.java:1843)
    at java.util.concurrent.ForkJoinPool.runWorker (ForkJoinPool.java:1808)
    at java.util.concurrent.ForkJoinWorkerThread.run (ForkJoinWorkerThread.java:188)

I would think the issue is the usage of JGitText inside the notify.

I would think hat this means is that all method of the ShutdownHook.java can not rely on JGitTextto be available.

@tomaswolf
Copy link
Contributor

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.

@tomaswolf
Copy link
Contributor

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:

  • clone https://github.com/seart-group/ghs
  • in the top-level pom, change the version of git-commit-id-maven-plugin from 8.0.1 to 8.0.0
  • run mvn clean package -DskipTests (needs a Java 17 JVM at least)
  • observe the exception
  • fix JGit, build and install JGit locally
  • in the ghs top-level pom, override the JGit dependency of git-commit-id-maven-plugin to 6.10.0-SNAPSHOT
  • run the ghs build again
  • exception is gone

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.

@laeubi
Copy link

laeubi commented Mar 11, 2024

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:

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.

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:

  1. In the (daemon) background thread write a new temp file with the new content.
  2. If content is written, now start a new (non daemon) thread, that deletes the original file and renames the temp file to the final destination.

@laeubi
Copy link

laeubi commented Mar 11, 2024

When using Tycho, in some circumstances Maven build fails with

Just for completeness this is the full stack trace now with 6.8.0.202311291450-r to 6.9.0.202403050737-r:

java.util.concurrent.ExecutionException: java.lang.NoClassDefFoundError
    at java.util.concurrent.ForkJoinTask.reportExecutionException (ForkJoinTask.java:605)
    at java.util.concurrent.ForkJoinTask.get (ForkJoinTask.java:1004)
    at org.eclipse.jgit.internal.util.ShutdownHook.cleanup (ShutdownHook.java:82)
    at java.lang.Thread.run (Thread.java:833)
Caused by: java.lang.NoClassDefFoundError
    at jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0 (Native Method)
    at jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance (NativeConstructorAccessorImpl.java:77)
    at jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance (DelegatingConstructorAccessorImpl.java:45)
    at java.lang.reflect.Constructor.newInstanceWithCaller (Constructor.java:499)
    at java.lang.reflect.Constructor.newInstance (Constructor.java:480)
    at java.util.concurrent.ForkJoinTask.getThrowableException (ForkJoinTask.java:564)
    at java.util.concurrent.ForkJoinTask.reportExecutionException (ForkJoinTask.java:604)
    at java.util.concurrent.ForkJoinTask.get (ForkJoinTask.java:1004)
    at org.eclipse.jgit.internal.util.ShutdownHook.cleanup (ShutdownHook.java:82)
    at java.lang.Thread.run (Thread.java:833)
Caused by: java.lang.NoClassDefFoundError: org/eclipse/jgit/internal/JGitText
    at org.eclipse.jgit.internal.util.ShutdownHook.notify (ShutdownHook.java:97)
    at java.util.stream.ForEachOps$ForEachOp$OfRef.accept (ForEachOps.java:183)
    at java.util.concurrent.ConcurrentHashMap$KeySpliterator.forEachRemaining (ConcurrentHashMap.java:3573)
    at java.util.stream.AbstractPipeline.copyInto (AbstractPipeline.java:509)
    at java.util.stream.ForEachOps$ForEachTask.compute (ForEachOps.java:290)
    at java.util.concurrent.CountedCompleter.exec (CountedCompleter.java:754)
    at java.util.concurrent.ForkJoinTask.doExec (ForkJoinTask.java:373)
    at java.util.concurrent.ForkJoinTask.invoke (ForkJoinTask.java:686)
    at java.util.stream.ForEachOps$ForEachOp.evaluateParallel (ForEachOps.java:159)
    at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateParallel (ForEachOps.java:173)
    at java.util.stream.AbstractPipeline.evaluate (AbstractPipeline.java:233)
    at java.util.stream.ReferencePipeline.forEach (ReferencePipeline.java:596)
    at java.util.stream.ReferencePipeline$Head.forEach (ReferencePipeline.java:765)
    at org.eclipse.jgit.internal.util.ShutdownHook.doCleanup (ShutdownHook.java:93)
    at org.eclipse.jgit.internal.util.ShutdownHook.lambda$1 (ShutdownHook.java:80)
    at java.util.concurrent.ForkJoinTask$AdaptedCallable.exec (ForkJoinTask.java:1428)
    at java.util.concurrent.ForkJoinTask.doExec (ForkJoinTask.java:373)
    at java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec (ForkJoinPool.java:1182)
    at java.util.concurrent.ForkJoinPool.scan (ForkJoinPool.java:1655)
    at java.util.concurrent.ForkJoinPool.runWorker (ForkJoinPool.java:1622)
    at java.util.concurrent.ForkJoinWorkerThread.run (ForkJoinWorkerThread.java:165)
Caused by: java.lang.ClassNotFoundException: org.eclipse.jgit.internal.JGitText
    at org.codehaus.plexus.classworlds.strategy.SelfFirstStrategy.loadClass (SelfFirstStrategy.java:50)
    at org.codehaus.plexus.classworlds.realm.ClassRealm.unsynchronizedLoadClass (ClassRealm.java:271)
    at org.codehaus.plexus.classworlds.realm.ClassRealm.loadClass (ClassRealm.java:247)
    at org.codehaus.plexus.classworlds.realm.ClassRealm.loadClass (ClassRealm.java:239)
    at org.eclipse.jgit.internal.util.ShutdownHook.notify (ShutdownHook.java:97)
    at java.util.stream.ForEachOps$ForEachOp$OfRef.accept (ForEachOps.java:183)
    at java.util.concurrent.ConcurrentHashMap$KeySpliterator.forEachRemaining (ConcurrentHashMap.java:3573)
    at java.util.stream.AbstractPipeline.copyInto (AbstractPipeline.java:509)
    at java.util.stream.ForEachOps$ForEachTask.compute (ForEachOps.java:290)
    at java.util.concurrent.CountedCompleter.exec (CountedCompleter.java:754)
    at java.util.concurrent.ForkJoinTask.doExec (ForkJoinTask.java:373)
    at java.util.concurrent.ForkJoinTask.invoke (ForkJoinTask.java:686)
    at java.util.stream.ForEachOps$ForEachOp.evaluateParallel (ForEachOps.java:159)
    at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateParallel (ForEachOps.java:173)
    at java.util.stream.AbstractPipeline.evaluate (AbstractPipeline.java:233)
    at java.util.stream.ReferencePipeline.forEach (ReferencePipeline.java:596)
    at java.util.stream.ReferencePipeline$Head.forEach (ReferencePipeline.java:765)
    at org.eclipse.jgit.internal.util.ShutdownHook.doCleanup (ShutdownHook.java:93)
    at org.eclipse.jgit.internal.util.ShutdownHook.lambda$1 (ShutdownHook.java:80)
    at java.util.concurrent.ForkJoinTask$AdaptedCallable.exec (ForkJoinTask.java:1428)
    at java.util.concurrent.ForkJoinTask.doExec (ForkJoinTask.java:373)
    at java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec (ForkJoinPool.java:1182)
    at java.util.concurrent.ForkJoinPool.scan (ForkJoinPool.java:1655)
    at java.util.concurrent.ForkJoinPool.runWorker (ForkJoinPool.java:1622)
    at java.util.concurrent.ForkJoinWorkerThread.run (ForkJoinWorkerThread.java:165)

I think all JGitText.get() calls must be called in the constructor or some static fields so at shutdownhook time there is no classloading happen anymore.

@laeubi
Copy link

laeubi commented Mar 11, 2024

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...

If there are different classloaders (e.g. in maven each mojo / project has its own) there will be multiple instances of "singleton" ShutdownHook and therefore multiple registration of Runtime.getRuntime().addShutdownHook(...). Shutdownhooks are really nasty and need to be used with care, there is even no guarantee they are executed at all, e.g. in eclipse if you stop the process with the red button the JVM is just terminated and never execute them.

@tomaswolf
Copy link
Contributor

Yes. As I wrote, pre-loading JGitText avoids this particular exception, but doesn't solve the other problems:

  1. If a cleanup listener executed through ShutdownHook needs to load other classes, we're still bust.
  2. In the maven build there may be many different instances of ShutdownHook, and execute all at the end, which is not what was intended.

Stale lock files are a problem, but it looks as if a shutdown hook cannot solve this.

@laeubi
Copy link

laeubi commented Mar 11, 2024

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

Runtime.getRuntime().addShutdownHook( ... );
Runtime.getRuntime().removeShutdownHook( ...);

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:

Why are hooks just threads, and unstarted ones at that? Wouldn't it be simpler to use Runnable objects, or Beans-style event and listener patterns?

The approach taken here has two advantages over the more obvious, and more frequently suggested, callback-oriented designs based upon Runnable objects or Beans-style event listeners.
First, it gives the user complete control over the thread upon which a shutdown action is executed. The thread can be created in the proper thread group, given the correct priority, context, and privileges, and so forth.
Second, it simplifies both the specification and the implementation by isolating the VM from the hooks themselves. If shutdown actions were executed as callbacks then a robust implementation would wind up having to create a separate thread for each hook anyway in order for them to run concurrently. The specification would also have to include explicit language about how the threads that execute the callbacks are created.

Aren't threads pretty expensive things to keep around, especially if they won't be started until the VM shuts down?

Most implementations of the Java platform don't actually allocate resources to a thread until it's started, so maintaining a set of unstarted threads is actually very cheap. If you look at the internals of java.lang.Thread you can see that its various constructors just do security checks and initialize private fields. The native start() method does the real work of allocating a thread stack, etc., to get things going.

@iloveeclipse
Copy link
Contributor

One point against using Runtime.getRuntime().addShutdownHook() in JGit could be that it might leak loaded data & loaded classes referenced by that shutdown thread, and so applications that use JGit in context of a dedicated classloader might leak the memory accessible from that classloader because it can't be properly garbage collected.

@laeubi
Copy link

laeubi commented Mar 11, 2024

@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 ShutdownHook class obsolete and not leak hooks or references.

@msohn
Copy link
Member

msohn commented Mar 11, 2024

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.

@laeubi
Copy link

laeubi commented Mar 11, 2024

@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.

@tomaswolf
Copy link
Contributor

For now solved by pre-loading JGitText.

@tomaswolf tomaswolf added this to the 6.10.0 milestone Mar 18, 2024
@LorenzoBettini
Copy link

I still see this problem in GitHub Actions builds.
Where should this have been solved?

@tomaswolf
Copy link
Contributor

In JGit 6.10.0, due June 12, 2024.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants