- Sponsor
-
Notifications
You must be signed in to change notification settings - Fork 70
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
Multiple runs and even builds for different projects influence each other #113
Comments
Actually with Actually without refactoring SpotBugs you would probably need an even tighter isolation mode that combines CLASSLOADER within PROCESS so that the stuff gets a separate classloader in the worker daemon. A working workaround that forces separate worker processes to be spawned is this: tasks.withType(SpotBugsTask) {
doFirst {
jvmArgs "-DworkerUniquifier=${UUID.randomUUID()}"
}
} As it runs in the execution phase, it does not influence the cache key for the build cache or the up-to-date check, but if the task will be run due to out-of-dateness and cache-miss, it will get its own exclusive worker process as on each invocation the JVM args will be different and so a new process needs to be spawned. |
#100 should fix this issue, we'll release next patch version soon so please try it later. |
Ah, yes, I see you switch to have one worker process per run, that should fix it, yes. |
Hm, do you have any estimate when "soon" will be approximately? |
I've commented on #100, I think it's not a good idea. Spotbugs should be fixed instead to not use static state. |
"soon" means right after #106 has been merged. I've requested @spotbugs/everyone to merge that.
Yes! It's my dearest for this several years and we're open for PR that realizes it. |
Running spotbugs in separate child worker JVMs kills parallel builds on any high-core-count system. This is slightly related to gradle/gradle#7047 but the real solution is to make spotbugs truly thread/instance-safe so we can run parallel builds internally. |
@Vampire now that this has been merged, it is still an issue? Please try using the latest version. |
please reopen if still an issue |
People cannot reopen tickets that you closed. |
Ok, I can reopen if you want. The merge I was referring to was the one @KengoTODA mentioned above #106. Please try using the latest version of the plugin and see if you still have an issue. If you do please create a minimum project that reproduces the issue. |
I don't see how #106 should be related to this issue. confused |
Oops, sorry I meant #100 (#113 (comment)) |
Ah, that one, I see. |
In the beta branch, we launch a |
SpotBugs uses static stuff which influences multiple runs and even builds for different projects and thus makes things different depending on situation and actually unpredictable.
Where I observed it is with spotbugs plugins as it gets pretty obviously and loudly displayed in this case, but there could of course be other and more subtle occurances that are not as obvious in effect.
Here two independent simple ways to reproduce the problem (simpler usage also triggers this, but with these instructions it should be 100% reproducible):
For all, add one or more spotbugs plugins, and make sure at least one bug will be found in each of the tasks run (somhow couldn't reproduce on first try without found bugs, didn't investigate why)
gradlew --stop
gradlew -i --daemon --max-workers 1 spotbugsMain
gradlew -i --daemon --max-workers 1 spotbugsMain
Trying to add already registered factory
warnings (if not, make sure that the last run had as one of its first linesStarting 2nd build in daemon
)gradlew -i --no-daemon --continue --max-workers 1 spotbugsMain spotbugsTest
spotbugsMain
will go like expectedspotbugsTest
you get a bunch ofTrying to add already registered factory
warningsThe cause here is, that the plugins are loaded with
DetectorFactoryCollection.instance().loadPlugin(plugin);
into a static singleton, so if the same worker JVM is reused, the previously registered plugins are still there.Besides the harmless warning (as long as the settings are the same), this of course also means that the plugins stay loaded, even if a different build does not want them.
This can happen with different SpotBugs tasks in the same project that are configured differently, but even worse it can happen with totally independent projects that are built in the same Gradle daemon where the SpotBugs tasks are run on the same worker JVMs.
In case of the
DetectorFactoryCollection
, it could maybe be worked around by usingedu.umd.cs.findbugs.DetectorFactoryCollection#resetInstance
, but according to its documentation itis public for tests only!
and maybe there are other stuff that is kept statically too.So I guess to fix this, either SpotBugs has to be refactored to not rely on static stuff or at least provide some means to reset all static stuff, or the isolation for the SpotBugs runner has to be increased, so that each SpotBugs run is done in its own forked JVM and not in reused worker JVMs. This will probably make the analysis a bit slower, but imho it is worth the wait if you in turn get more reliable, reproducable and expected results. The best would of course be the first option, to make SpotBugs properly work with different settings in the same JVM mutliple times, but as this is not a problem with "normal" SpotBugs usage per-se, I thought here might be the more appropriate place to report it.
The text was updated successfully, but these errors were encountered: