[JENKINS-19453] Interrupted class loading can lead to NoClassDefFoundError #19
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I get what I think is the expected stack trace when the new test fails: |
|
I feel like the right solution would be that when we catch |
|
Tried without success. |
| } | ||
| } | ||
| }); | ||
| Thread.sleep(2500); |
There was a problem hiding this comment.
This is so that the first two class loads succeed but the third fails. A better test would use semaphores rather than timing (cf. the test before this one).
|
Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests |
|
So the problem here is that if the thread gets interrupted while |
I think the crux of the issue is that, in general, a failed class loading will never get retried. It is certainly the case when another class loading has triggered a recurisve class loading as in this test case. In #19 Jesse proposed to fix this by dropping the whole RemoteClassLoader. But this will not work since the said RemoteClassLoader cloud have loaded other classes that might be already running. Dropping a classloader will not drop these classes, and we end up just creating another classloader that loads incompatible classes. That is a disaster waiting to happen. In this fix, we simply make findClass non-interruptible. That is, if a blocking remote operation gets interrupted, we catch that and simply retry and refuse to give up. We'll remember to set the interrupt flag back on, however, so that the interrupt signal doesn't disappear. The net effect is as if the delivery of the interrupt was bit late than usual. The way I see it, this is the only way to fix this. The nesting of control structures here is horrible, but I can't think of any better way to write this.
| Future<byte[]> img = cr.classImage.resolve(channel, name.replace('.', '/') + ".class"); | ||
| if (img.isDone()) { | ||
| boolean interrupted = false; | ||
| try {// |
There was a problem hiding this comment.
Did you mean to add a comment here?
|
I think your approach is right. Since unfortunately Java class loading is not atomic with respect to static initializers—i.e. the JVM fails to cleanly roll back to the prior state if some classes are loaded but then an initializer blocks loading of one—probably the best we can do is to just doggedly force the class to be loaded. |
kohsuke
added a commit
that referenced
this pull request
Nov 11, 2013
I think the crux of the issue is that, in general, a failed class loading will never get retried. It is certainly the case when another class loading has triggered a recurisve class loading as in this test case. In #19 Jesse proposed to fix this by dropping the whole RemoteClassLoader. But this will not work since the said RemoteClassLoader cloud have loaded other classes that might be already running. Dropping a classloader will not drop these classes, and we end up just creating another classloader that loads incompatible classes. That is a disaster waiting to happen. In this fix, we simply make findClass non-interruptible. That is, if a blocking remote operation gets interrupted, we catch that and simply retry and refuse to give up. We'll remember to set the interrupt flag back on, however, so that the interrupt signal doesn't disappear. The net effect is as if the delivery of the interrupt was bit late than usual. The way I see it, this is the only way to fix this. The nesting of control structures here is horrible, but I can't think of any better way to write this.
|
Rebased and integrated toward 2.33. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
JENKINS-19453: interrupting class loading irrecoverably breaks the class being loaded. Filing for evaluation; so far just have a failing test.