You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "Pankraz76 (via GitHub)" <gi...@apache.org> on 2025/05/23 07:15:18 UTC
[PR] extract dedicated class `CheckThreadSafetyThread` from `CheckThreadSafetyMojo` for concern `checkThreadSafety` [maven]
Pankraz76 opened a new pull request, #2382:
URL: https://github.com/apache/maven/pull/2382
- https://github.com/apache/maven/pull/2381
Following this checklist to help us incorporate your
contribution quickly and easily:
- [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/MNG) filed
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
- [ ] Each commit in the pull request should have a meaningful subject line and body.
- [ ] Format the pull request title like `[MNG-XXX] SUMMARY`,
where you replace `MNG-XXX` and `SUMMARY` with the appropriate JIRA issue.
- [ ] Also format the first line of the commit message like `[MNG-XXX] SUMMARY`.
Best practice is to use the JIRA issue title in both the pull request title and in the first line of the commit message.
- [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [ ] Run `mvn clean verify` to make sure basic checks pass. A more thorough check will
be performed on your pull request automatically.
- [ ] You have run the [Core IT][core-its] successfully.
If your pull request is about ~20 lines of code you don't need to sign an
[Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [ ] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
- [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
[core-its]: https://maven.apache.org/core-its/core-it-suite/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] dedicate class `CheckThreadSafetyThread` from `CheckThreadSafetyMojo` for concern `checkThreadSafety` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2382:
URL: https://github.com/apache/maven/pull/2382#discussion_r2103993970
##########
its/core-it-support/core-it-plugins/maven-it-plugin-active-collection/src/main/java/org/apache/maven/plugin/coreit/CheckThreadSafetyMojo.java:
##########
@@ -70,86 +71,97 @@ public class CheckThreadSafetyMojo extends AbstractMojo {
* Runs this mojo.
*
* @throws MojoExecutionException If the output file could not be created.
+ *
+ * @implNote threads need to use different realms to trigger changes of the collections.
*/
public void execute() throws MojoExecutionException {
- Properties componentProperties = new Properties();
-
getLog().info("[MAVEN-CORE-IT-LOG] Testing concurrent component access");
-
- ClassLoader pluginRealm = getClass().getClassLoader();
- ClassLoader coreRealm = MojoExecutionException.class.getClassLoader();
-
- final Map map = componentMap;
- final List list = componentList;
- final List go = new Vector();
- final List exceptions = new Vector();
-
- Thread[] threads = new Thread[2];
+ final List<Exception> exceptions = new Vector<>();
+ final CountDownLatch startLatch = new CountDownLatch(1);
+ final Thread[] threads = new Thread[2];
for (int i = 0; i < threads.length; i++) {
- // NOTE: The threads need to use different realms to trigger changes of the collections
- final ClassLoader cl = (i % 2) == 0 ? pluginRealm : coreRealm;
- threads[i] = new Thread() {
- private final ClassLoader tccl = cl;
-
- public void run() {
- getLog().info("[MAVEN-CORE-IT-LOG] Thread " + this + " uses " + tccl);
- Thread.currentThread().setContextClassLoader(tccl);
- while (go.isEmpty()) {
- // wait for start
- }
- for (int j = 0; j < 10 * 1000; j++) {
- try {
- for (Object o : map.values()) {
- o.toString();
- }
- for (Object aList : list) {
- aList.toString();
- }
- } catch (Exception e) {
- getLog().warn("[MAVEN-CORE-IT-LOG] Thread " + this + " encountered concurrency issue", e);
- exceptions.add(e);
- }
- }
- }
- };
+ threads[i] = new CheckThreadSafetyThread((i % 2) == 0 ?
Review Comment:
```suggestion
threads[i] = new CheckThreadSafetyThread(((i % 2)) == (0) ?
```
why there is no [UnnecessaryParentheses](https://checkstyle.sourceforge.io/checks/coding/unnecessaryparentheses.html) on 0?
now I see, where the problem at. Thanks check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] dedicate class `CheckThreadSafetyThread` from `CheckThreadSafetyMojo` for concern `checkThreadSafety` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2382:
URL: https://github.com/apache/maven/pull/2382#discussion_r2104197867
##########
its/core-it-support/core-it-plugins/maven-it-plugin-active-collection/src/main/java/org/apache/maven/plugin/coreit/CheckThreadSafetyMojo.java:
##########
@@ -66,64 +68,50 @@ public class CheckThreadSafetyMojo extends AbstractMojo {
@Parameter(property = "collections.outputFile")
private File outputFile;
+
/**
* Runs this mojo.
*
* @throws MojoExecutionException If the output file could not be created.
+ *
+ * @implNote threads need to use different realms to trigger changes of the collections.
*/
public void execute() throws MojoExecutionException {
Review Comment:
test for test seems strange.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] dedicate class `CheckThreadSafetyThread` from `CheckThreadSafetyMojo` for concern `checkThreadSafety` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2382:
URL: https://github.com/apache/maven/pull/2382#discussion_r2104013832
##########
its/core-it-support/core-it-plugins/maven-it-plugin-active-collection/src/main/java/org/apache/maven/plugin/coreit/CheckThreadSafetyMojo.java:
##########
@@ -70,86 +71,97 @@ public class CheckThreadSafetyMojo extends AbstractMojo {
* Runs this mojo.
*
* @throws MojoExecutionException If the output file could not be created.
+ *
+ * @implNote threads need to use different realms to trigger changes of the collections.
*/
public void execute() throws MojoExecutionException {
- Properties componentProperties = new Properties();
-
getLog().info("[MAVEN-CORE-IT-LOG] Testing concurrent component access");
-
- ClassLoader pluginRealm = getClass().getClassLoader();
- ClassLoader coreRealm = MojoExecutionException.class.getClassLoader();
-
- final Map map = componentMap;
- final List list = componentList;
- final List go = new Vector();
- final List exceptions = new Vector();
-
- Thread[] threads = new Thread[2];
+ final List<Exception> exceptions = new Vector<>();
+ final CountDownLatch startLatch = new CountDownLatch(1);
+ final Thread[] threads = new Thread[2];
for (int i = 0; i < threads.length; i++) {
- // NOTE: The threads need to use different realms to trigger changes of the collections
- final ClassLoader cl = (i % 2) == 0 ? pluginRealm : coreRealm;
- threads[i] = new Thread() {
Review Comment:
why no runnable? we only implement run method so this is kind of funny.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] dedicate class `CheckThreadSafetyThread` from `CheckThreadSafetyMojo` for concern `checkThreadSafety` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on PR #2382:
URL: https://github.com/apache/maven/pull/2382#issuecomment-2905626972
wip
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] dedicate class `CheckThreadSafetyThread` from `CheckThreadSafetyMojo` for concern `checkThreadSafety` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2382:
URL: https://github.com/apache/maven/pull/2382#discussion_r2103998803
##########
its/core-it-support/core-it-plugins/maven-it-plugin-active-collection/src/main/java/org/apache/maven/plugin/coreit/CheckThreadSafetyMojo.java:
##########
@@ -70,86 +71,97 @@ public class CheckThreadSafetyMojo extends AbstractMojo {
* Runs this mojo.
*
* @throws MojoExecutionException If the output file could not be created.
+ *
+ * @implNote threads need to use different realms to trigger changes of the collections.
*/
public void execute() throws MojoExecutionException {
- Properties componentProperties = new Properties();
-
getLog().info("[MAVEN-CORE-IT-LOG] Testing concurrent component access");
-
- ClassLoader pluginRealm = getClass().getClassLoader();
- ClassLoader coreRealm = MojoExecutionException.class.getClassLoader();
-
- final Map map = componentMap;
- final List list = componentList;
- final List go = new Vector();
- final List exceptions = new Vector();
-
- Thread[] threads = new Thread[2];
+ final List<Exception> exceptions = new Vector<>();
+ final CountDownLatch startLatch = new CountDownLatch(1);
+ final Thread[] threads = new Thread[2];
for (int i = 0; i < threads.length; i++) {
- // NOTE: The threads need to use different realms to trigger changes of the collections
- final ClassLoader cl = (i % 2) == 0 ? pluginRealm : coreRealm;
- threads[i] = new Thread() {
- private final ClassLoader tccl = cl;
-
- public void run() {
- getLog().info("[MAVEN-CORE-IT-LOG] Thread " + this + " uses " + tccl);
- Thread.currentThread().setContextClassLoader(tccl);
- while (go.isEmpty()) {
- // wait for start
- }
- for (int j = 0; j < 10 * 1000; j++) {
- try {
- for (Object o : map.values()) {
- o.toString();
- }
- for (Object aList : list) {
- aList.toString();
- }
- } catch (Exception e) {
- getLog().warn("[MAVEN-CORE-IT-LOG] Thread " + this + " encountered concurrency issue", e);
- exceptions.add(e);
- }
- }
- }
- };
+ threads[i] = new CheckThreadSafetyThread((i % 2) == 0 ?
Review Comment:
Im curious about the pattern, whats the higher call? While considered misleading in sonar conventions therefore applied for all languages in various code analysis tools.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] dedicate class `CheckThreadSafetyThread` from `CheckThreadSafetyMojo` for concern `checkThreadSafety` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2382:
URL: https://github.com/apache/maven/pull/2382#discussion_r2103993970
##########
its/core-it-support/core-it-plugins/maven-it-plugin-active-collection/src/main/java/org/apache/maven/plugin/coreit/CheckThreadSafetyMojo.java:
##########
@@ -70,86 +71,97 @@ public class CheckThreadSafetyMojo extends AbstractMojo {
* Runs this mojo.
*
* @throws MojoExecutionException If the output file could not be created.
+ *
+ * @implNote threads need to use different realms to trigger changes of the collections.
*/
public void execute() throws MojoExecutionException {
- Properties componentProperties = new Properties();
-
getLog().info("[MAVEN-CORE-IT-LOG] Testing concurrent component access");
-
- ClassLoader pluginRealm = getClass().getClassLoader();
- ClassLoader coreRealm = MojoExecutionException.class.getClassLoader();
-
- final Map map = componentMap;
- final List list = componentList;
- final List go = new Vector();
- final List exceptions = new Vector();
-
- Thread[] threads = new Thread[2];
+ final List<Exception> exceptions = new Vector<>();
+ final CountDownLatch startLatch = new CountDownLatch(1);
+ final Thread[] threads = new Thread[2];
for (int i = 0; i < threads.length; i++) {
- // NOTE: The threads need to use different realms to trigger changes of the collections
- final ClassLoader cl = (i % 2) == 0 ? pluginRealm : coreRealm;
- threads[i] = new Thread() {
- private final ClassLoader tccl = cl;
-
- public void run() {
- getLog().info("[MAVEN-CORE-IT-LOG] Thread " + this + " uses " + tccl);
- Thread.currentThread().setContextClassLoader(tccl);
- while (go.isEmpty()) {
- // wait for start
- }
- for (int j = 0; j < 10 * 1000; j++) {
- try {
- for (Object o : map.values()) {
- o.toString();
- }
- for (Object aList : list) {
- aList.toString();
- }
- } catch (Exception e) {
- getLog().warn("[MAVEN-CORE-IT-LOG] Thread " + this + " encountered concurrency issue", e);
- exceptions.add(e);
- }
- }
- }
- };
+ threads[i] = new CheckThreadSafetyThread((i % 2) == 0 ?
Review Comment:
```suggestion
threads[i] = new CheckThreadSafetyThread((((i % 2))) == (0) ?
```
why there is no [UnnecessaryParentheses](https://checkstyle.sourceforge.io/checks/coding/unnecessaryparentheses.html) on 0?
now I see, where the problem at. Thanks check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] dedicate class `CheckThreadSafetyThread` from `CheckThreadSafetyMojo` for concern `checkThreadSafety` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2382:
URL: https://github.com/apache/maven/pull/2382#discussion_r2103993970
##########
its/core-it-support/core-it-plugins/maven-it-plugin-active-collection/src/main/java/org/apache/maven/plugin/coreit/CheckThreadSafetyMojo.java:
##########
@@ -70,86 +71,97 @@ public class CheckThreadSafetyMojo extends AbstractMojo {
* Runs this mojo.
*
* @throws MojoExecutionException If the output file could not be created.
+ *
+ * @implNote threads need to use different realms to trigger changes of the collections.
*/
public void execute() throws MojoExecutionException {
- Properties componentProperties = new Properties();
-
getLog().info("[MAVEN-CORE-IT-LOG] Testing concurrent component access");
-
- ClassLoader pluginRealm = getClass().getClassLoader();
- ClassLoader coreRealm = MojoExecutionException.class.getClassLoader();
-
- final Map map = componentMap;
- final List list = componentList;
- final List go = new Vector();
- final List exceptions = new Vector();
-
- Thread[] threads = new Thread[2];
+ final List<Exception> exceptions = new Vector<>();
+ final CountDownLatch startLatch = new CountDownLatch(1);
+ final Thread[] threads = new Thread[2];
for (int i = 0; i < threads.length; i++) {
- // NOTE: The threads need to use different realms to trigger changes of the collections
- final ClassLoader cl = (i % 2) == 0 ? pluginRealm : coreRealm;
- threads[i] = new Thread() {
- private final ClassLoader tccl = cl;
-
- public void run() {
- getLog().info("[MAVEN-CORE-IT-LOG] Thread " + this + " uses " + tccl);
- Thread.currentThread().setContextClassLoader(tccl);
- while (go.isEmpty()) {
- // wait for start
- }
- for (int j = 0; j < 10 * 1000; j++) {
- try {
- for (Object o : map.values()) {
- o.toString();
- }
- for (Object aList : list) {
- aList.toString();
- }
- } catch (Exception e) {
- getLog().warn("[MAVEN-CORE-IT-LOG] Thread " + this + " encountered concurrency issue", e);
- exceptions.add(e);
- }
- }
- }
- };
+ threads[i] = new CheckThreadSafetyThread((i % 2) == 0 ?
Review Comment:
```suggestion
threads[i] = new CheckThreadSafetyThread((i % 2) == (0) ?
```
why there is no [UnnecessaryParentheses](https://checkstyle.sourceforge.io/checks/coding/unnecessaryparentheses.html) on 0?
now I see, where the problem at. Thanks check.
##########
its/core-it-support/core-it-plugins/maven-it-plugin-active-collection/src/main/java/org/apache/maven/plugin/coreit/CheckThreadSafetyMojo.java:
##########
@@ -70,86 +71,97 @@ public class CheckThreadSafetyMojo extends AbstractMojo {
* Runs this mojo.
*
* @throws MojoExecutionException If the output file could not be created.
+ *
+ * @implNote threads need to use different realms to trigger changes of the collections.
*/
public void execute() throws MojoExecutionException {
- Properties componentProperties = new Properties();
-
getLog().info("[MAVEN-CORE-IT-LOG] Testing concurrent component access");
-
- ClassLoader pluginRealm = getClass().getClassLoader();
- ClassLoader coreRealm = MojoExecutionException.class.getClassLoader();
-
- final Map map = componentMap;
- final List list = componentList;
- final List go = new Vector();
- final List exceptions = new Vector();
-
- Thread[] threads = new Thread[2];
+ final List<Exception> exceptions = new Vector<>();
+ final CountDownLatch startLatch = new CountDownLatch(1);
+ final Thread[] threads = new Thread[2];
for (int i = 0; i < threads.length; i++) {
- // NOTE: The threads need to use different realms to trigger changes of the collections
- final ClassLoader cl = (i % 2) == 0 ? pluginRealm : coreRealm;
- threads[i] = new Thread() {
- private final ClassLoader tccl = cl;
-
- public void run() {
- getLog().info("[MAVEN-CORE-IT-LOG] Thread " + this + " uses " + tccl);
Review Comment:
this comes from the logger normally, right? its kind of dry:
<img width="400" alt="image" src="https://github.com/user-attachments/assets/745ee741-b950-4368-a9bd-1de65b2b31a7" />
##########
its/core-it-support/core-it-plugins/maven-it-plugin-active-collection/src/main/java/org/apache/maven/plugin/coreit/CheckThreadSafetyMojo.java:
##########
@@ -70,86 +71,97 @@ public class CheckThreadSafetyMojo extends AbstractMojo {
* Runs this mojo.
*
* @throws MojoExecutionException If the output file could not be created.
+ *
+ * @implNote threads need to use different realms to trigger changes of the collections.
*/
public void execute() throws MojoExecutionException {
- Properties componentProperties = new Properties();
-
getLog().info("[MAVEN-CORE-IT-LOG] Testing concurrent component access");
-
- ClassLoader pluginRealm = getClass().getClassLoader();
- ClassLoader coreRealm = MojoExecutionException.class.getClassLoader();
-
- final Map map = componentMap;
- final List list = componentList;
- final List go = new Vector();
- final List exceptions = new Vector();
-
- Thread[] threads = new Thread[2];
+ final List<Exception> exceptions = new Vector<>();
+ final CountDownLatch startLatch = new CountDownLatch(1);
+ final Thread[] threads = new Thread[2];
for (int i = 0; i < threads.length; i++) {
- // NOTE: The threads need to use different realms to trigger changes of the collections
Review Comment:
might be candidate for @implNote to give special attention, thus higher scope.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] dedicate class `CheckThreadSafetyThread` from `CheckThreadSafetyMojo` for concern `checkThreadSafety` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 closed pull request #2382: dedicate class `CheckThreadSafetyThread` from `CheckThreadSafetyMojo` for concern `checkThreadSafety`
URL: https://github.com/apache/maven/pull/2382
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] dedicate class `CheckThreadSafetyThread` from `CheckThreadSafetyMojo` for concern `checkThreadSafety` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2382:
URL: https://github.com/apache/maven/pull/2382#discussion_r2104022264
##########
its/core-it-support/core-it-plugins/maven-it-plugin-active-collection/src/main/java/org/apache/maven/plugin/coreit/CheckThreadSafetyMojo.java:
##########
@@ -66,64 +68,50 @@ public class CheckThreadSafetyMojo extends AbstractMojo {
@Parameter(property = "collections.outputFile")
private File outputFile;
+
/**
* Runs this mojo.
*
* @throws MojoExecutionException If the output file could not be created.
+ *
+ * @implNote threads need to use different realms to trigger changes of the collections.
*/
public void execute() throws MojoExecutionException {
Review Comment:
require test coverage.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org