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

Not yet compatible with Maven parallel execution #161

Closed
sgammon opened this issue Mar 9, 2024 · 5 comments · Fixed by #163
Closed

Not yet compatible with Maven parallel execution #161

sgammon opened this issue Mar 9, 2024 · 5 comments · Fixed by #163

Comments

@sgammon
Copy link

sgammon commented Mar 9, 2024

When executing Maven with -T 1C:

[WARNING] *****************************************************************
[WARNING] * Your build is requesting parallel execution, but this         *
[WARNING] * project contains the following plugin(s) that have goals not  *
[WARNING] * marked as thread-safe to support parallel execution.          *
[WARNING] * While this /may/ work fine, please look for plugin updates    *
[WARNING] * and/or request plugins be made thread-safe.                   *
[WARNING] * If reporting an issue, report it against the plugin in        *
[WARNING] * question, not against Apache Maven.                           *
[WARNING] *****************************************************************
[WARNING] The following plugins are not marked as thread-safe in Guava: Google Core Libraries for Java:
[WARNING]   org.spdx:spdx-maven-plugin:0.7.3
[WARNING] 
[WARNING] Enable debug to see precisely which goals are not marked as thread-safe.
[WARNING] *****************************************************************
@goneall
Copy link
Member

goneall commented Mar 10, 2024

Thanks @sgammon for reporting this. The underlying SPDX libraries used by the maven plugin are thread-safe, however, we should review the plugin code itself to see if it is implemented in a thread safe manner.

@sgammon
Copy link
Author

sgammon commented Mar 10, 2024

@goneall For posterity's sake, the SPDX goal has not failed even once yet and it's part of my regular build (multi-threading on)

@goneall
Copy link
Member

goneall commented Mar 11, 2024

I found one potential issue - MavenToSpdxLicenseMapper is a singleton class which is not using any locking when creating the static instance - should be a relatively straightforward fix. I'll create a PR.

Additional analysis following the advice in this stackoverflow article:

Check all static fields/variables in plugin/plugin code are not subject to threading problems.

Most are final simple types with the exception of the Logger, Pattern.compile (which claims to be thread-safe), static HashMap s which are initialized in static blocks, and the above referenced singleton class.

Check any plexus components.xml; if the components defined are singletons they need to be thread-safe.

None found

Check for presence of known tainted libraries.

The SPDX libraries are threadsafe and the other dependencies are all related to Maven - so I'm assuming they are threadsafe

@goneall
Copy link
Member

goneall commented Mar 11, 2024

Actually - after thinking about the design of MavenToSpdxLicenseMapper - I don't think there would be much impact to having 2 (or more) instances created if the window is hit. That being said, I'll still create a PR to make it clear that the code is threadsafe.

goneall added a commit that referenced this issue Mar 11, 2024
Fixes #161

Signed-off-by: Gary O'Neall <gary@sourceauditor.com>
goneall added a commit that referenced this issue Mar 11, 2024
Fixes #161

Signed-off-by: Gary O'Neall <gary@sourceauditor.com>
@sgammon
Copy link
Author

sgammon commented Mar 11, 2024

Nice! If you need a tester downstream, I'm adding this plugin to Guava.

goneall added a commit that referenced this issue Mar 11, 2024
Fixes #161

Signed-off-by: Gary O'Neall <gary@sourceauditor.com>
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

Successfully merging a pull request may close this issue.

2 participants