Skip to content

dedicate class CheckThreadSafetyThread from CheckThreadSafetyMojo for concern checkThreadSafety #2382

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.

Sign up for GitHub

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

Closed
Pankraz76 wants to merge 1 commit into from
Closed

dedicate class CheckThreadSafetyThread from CheckThreadSafetyMojo for concern checkThreadSafety #2382

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 68 additions & 54 deletions ...active-collection/src/main/java/org/apache/maven/plugin/coreit/CheckThreadSafetyMojo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,15 @@
import java.util.Map;
import java.util.Properties;
import java.util.Vector;
import java.util.concurrent.CountDownLatch;

import org.apache.maven.plugin.AbstractMojo;
import org.apache.maven.plugin.MojoExecutionException;
import org.apache.maven.plugins.annotations.Component;
import org.apache.maven.plugins.annotations.LifecyclePhase;
import org.apache.maven.plugins.annotations.Mojo;
import org.apache.maven.plugins.annotations.Parameter;
import org.apache.maven.plugin.logging.Log;

/**
* Checks the thread-safe retrieval of components from active component collections.
Expand All @@ -42,6 +44,7 @@
@Mojo(name = "check-thread-safety", defaultPhase = LifecyclePhase.VALIDATE)
public class CheckThreadSafetyMojo extends AbstractMojo {

private static final String MAVEN_CORE_IT_LOG = "[MAVEN-CORE-IT-LOG] ";
/**
* Project base directory used for manual path alignment.
*/
Expand Down Expand Up @@ -70,86 +73,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 {
Copy link
Contributor Author

@Pankraz76 Pankraz76 May 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

require test coverage.

Copy link
Contributor Author

@Pankraz76 Pankraz76 May 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test for test seems strange.

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];
getLog().info(MAVEN_CORE_IT_LOG + "Testing concurrent component access");
final Thread[] threads = new Thread[2];
final List<Exception> exceptions = new Vector<>();
final CountDownLatch startLatch = new CountDownLatch(1);
for (int i = 0; i < threads.length; i++) {
// NOTE: The threads need to use different realms to trigger changes of the collections
Copy link
Contributor Author

@Pankraz76 Pankraz76 May 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be candidate for @implNote to give special attention, thus higher scope.

final ClassLoader cl = (i % 2) == 0 ? pluginRealm : coreRealm;
threads[i] = new Thread() {
Copy link
Contributor Author

@Pankraz76 Pankraz76 May 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why no runnable? we only implement run method so this is kind of funny.

private final ClassLoader tccl = cl;

public void run() {
getLog().info("[MAVEN-CORE-IT-LOG] Thread " + this + " uses " + tccl);
Copy link
Contributor Author

@Pankraz76 Pankraz76 May 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comes from the logger normally, right? its kind of dry:

image

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 Thread(new CheckThreadSafetyTask(
(i % 2) == 0 ? getClass().getClassLoader() : MojoExecutionException.class.getClassLoader(),
startLatch,
componentMap,
componentList,
exceptions,
getLog()
));
threads[i].start();
}

go.add(null);
startLatch.countDown(); // signal all threads to start
joinOrInterrupt(threads);
storeComponentProperties(exceptions);
getLog().info(MAVEN_CORE_IT_LOG + "Created output file " + outputFile);
}

private void joinOrInterrupt(Thread[] threads) {
for (Thread thread : threads) {
try {
thread.join();
} catch (InterruptedException e) {
getLog().warn("[MAVEN-CORE-IT-LOG] Interrupted while joining " + thread);
Thread.currentThread().interrupt();
getLog().warn(MAVEN_CORE_IT_LOG + "Interrupted while joining " + thread);
}
}
}

private void storeComponentProperties( List<Exception> exceptions) throws MojoExecutionException {
final Properties componentProperties= new Properties();
componentProperties.setProperty("components", Integer.toString(componentList.size()));
componentProperties.setProperty("exceptions", Integer.toString(exceptions.size()));

if (!outputFile.isAbsolute()) {
outputFile = new File(basedir, outputFile.getPath());
}

getLog().info("[MAVEN-CORE-IT-LOG] Creating output file " + outputFile);

OutputStream out = null;
try {
getLog().info(MAVEN_CORE_IT_LOG + "Creating output file " + outputFile);
try (OutputStream out = new FileOutputStream(outputFile)) {
outputFile.getParentFile().mkdirs();
out = new FileOutputStream(outputFile);
componentProperties.store(out, "MAVEN-CORE-IT-LOG");
} catch (IOException e) {
throw new MojoExecutionException("Output file could not be created: " + outputFile, e);
} finally {
if (out != null) {
}
}

private record CheckThreadSafetyTask(
ClassLoader tccl,
CountDownLatch startLatch,
Map<String, TestComponent> map,
List<TestComponent> list,
List<Exception> exceptions,
Log log
) implements Runnable {

@Override
public void run() {
log.info(MAVEN_CORE_IT_LOG + "Thread " + Thread.currentThread() + " uses " + tccl);
Thread.currentThread().setContextClassLoader(tccl);
try {
startLatch.await(); // wait for the start signal
checkThreadSafety();
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
log.warn(MAVEN_CORE_IT_LOG + "Thread " + Thread.currentThread() + " was interrupted while waiting");
}
}

private void checkThreadSafety() {
for (int j = 0; j < 10 * 1000; j++) {
try {
out.close();
} catch (IOException e) {
// just ignore
for (Object o : map.values()) {
o.toString();
}
for (Object aList : list) {
aList.toString();
}
} catch (Exception e) {
log.warn(MAVEN_CORE_IT_LOG + "Thread " + Thread.currentThread() + " encountered concurrency issue", e);
exceptions.add(e);
}
}
}

getLog().info("[MAVEN-CORE-IT-LOG] Created output file " + outputFile);
}
}
}