From 02132bd8cada08cad00e2a51f1f3cc44493125fc Mon Sep 17 00:00:00 2001 From: Krishnan Mahadevan Date: Mon, 26 Feb 2024 08:52:06 +0530 Subject: [PATCH 1/4] Use Locks instead of synchronised keyword Closes #3040 --- CHANGES.txt | 1 + .../src/main/java/org/testng/Reporter.java | 21 ++- .../testng/internal/AutoCloseableLock.java | 23 +++ .../internal/KeyAwareAutoCloseableLock.java | 37 ++++ .../main/java/org/testng/SkipException.java | 8 +- .../src/main/java/org/testng/SuiteRunner.java | 4 +- .../internal/ConfigurationGroupMethods.java | 10 +- .../internal/invokers/ConfigInvoker.java | 5 +- .../testng/internal/invokers/TestInvoker.java | 74 ++++---- .../internal/invokers/TestMethodWorker.java | 7 +- .../thread/graph/GraphOrchestrator.java | 9 +- .../thread/graph/GraphThreadPoolExecutor.java | 171 ------------------ .../testng/reporters/JUnitXMLReporter.java | 28 +-- .../main/java/org/testng/xml/XMLParser.java | 5 +- .../ConfigurationGroupBothSampleTest.java | 9 +- .../issue2489/tests/BaseClassA.java | 8 +- .../TestClassContainerForGithubIssue1265.java | 4 +- .../listeners/factory/ExampleListener.java | 11 +- .../issue2043/listeners/FailFastListener.java | 6 +- .../issue2798/HashCodeAwareRetryAnalyzer.java | 11 +- .../test/testng1232/TestListenerFor1232.java | 8 +- .../test/java/test/thread/BaseThreadTest.java | 13 +- .../MultiThreadedDependentSampleTest.java | 7 +- .../thread/MultiThreadedDependentTest.java | 4 +- 24 files changed, 229 insertions(+), 255 deletions(-) create mode 100644 testng-core-api/src/main/java/org/testng/internal/AutoCloseableLock.java create mode 100644 testng-core-api/src/main/java/org/testng/internal/KeyAwareAutoCloseableLock.java delete mode 100644 testng-core/src/main/java/org/testng/internal/thread/graph/GraphThreadPoolExecutor.java diff --git a/CHANGES.txt b/CHANGES.txt index 71cc122c51..a1729e74b2 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ Current (7.10.0) +Fixed: GITHUB:3040: replace the usages of synchronized with ReentrantLock (Krishnan Mahadevan) Fixed: GITHUB-3041: TestNG 7.x DataProvider works in opposite to TestNG 6.x when retrying tests. (Krishnan Mahadevan) Fixed: GITHUB-3066: How to dynamically adjust the number of TestNG threads after IExecutorFactory is deprecated? (Krishnan Mahadevan) New: GITHUB-2874: Allow users to define ordering for TestNG listeners (Krishnan Mahadevan) diff --git a/testng-core-api/src/main/java/org/testng/Reporter.java b/testng-core-api/src/main/java/org/testng/Reporter.java index e27356b6db..ee322b8122 100644 --- a/testng-core-api/src/main/java/org/testng/Reporter.java +++ b/testng-core-api/src/main/java/org/testng/Reporter.java @@ -5,6 +5,7 @@ import java.util.Map; import org.testng.collections.Lists; import org.testng.collections.Maps; +import org.testng.internal.AutoCloseableLock; import org.testng.internal.Utils; import org.testng.util.Strings; @@ -67,7 +68,15 @@ public static void setEscapeHtml(boolean escapeHtml) { m_escapeHtml = escapeHtml; } - private static synchronized void log(String s, ITestResult m) { + private static final AutoCloseableLock lockForLogging = new AutoCloseableLock(); + + private static void log(String s, ITestResult m) { + try (AutoCloseableLock ignore = lockForLogging.lock()) { + logToReports(s, m); + } + } + + private static void logToReports(String s, ITestResult m) { // Escape for the HTML reports. if (m_escapeHtml) { s = Strings.escapeHtml(s); @@ -157,7 +166,15 @@ public static ITestResult getCurrentTestResult() { return m_currentTestResult.get(); } - public static synchronized List getOutput(ITestResult tr) { + private static final AutoCloseableLock lockForOutputs = new AutoCloseableLock(); + + public static List getOutput(ITestResult tr) { + try (AutoCloseableLock ignore = lockForOutputs.lock()) { + return getOutputFromResult(tr); + } + } + + private static List getOutputFromResult(ITestResult tr) { List result = Lists.newArrayList(); if (tr == null) { // Guard against a possible NPE in scenarios wherein the test result object itself could be a diff --git a/testng-core-api/src/main/java/org/testng/internal/AutoCloseableLock.java b/testng-core-api/src/main/java/org/testng/internal/AutoCloseableLock.java new file mode 100644 index 0000000000..bb4e612246 --- /dev/null +++ b/testng-core-api/src/main/java/org/testng/internal/AutoCloseableLock.java @@ -0,0 +1,23 @@ +package org.testng.internal; + +import java.io.Closeable; +import java.util.concurrent.locks.ReentrantLock; + +/** + * A simple abstraction over {@link ReentrantLock} that can be used in conjunction with + * try..resources constructs. + */ +public final class AutoCloseableLock implements Closeable { + + private final ReentrantLock internalLock = new ReentrantLock(); + + public AutoCloseableLock lock() { + internalLock.lock(); + return this; + } + + @Override + public void close() { + internalLock.unlock(); + } +} diff --git a/testng-core-api/src/main/java/org/testng/internal/KeyAwareAutoCloseableLock.java b/testng-core-api/src/main/java/org/testng/internal/KeyAwareAutoCloseableLock.java new file mode 100644 index 0000000000..348d9d85cf --- /dev/null +++ b/testng-core-api/src/main/java/org/testng/internal/KeyAwareAutoCloseableLock.java @@ -0,0 +1,37 @@ +package org.testng.internal; + +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; + +/** + * A simple abstraction over {@link java.util.concurrent.locks.ReentrantLock} that can be used when + * we need to be dealing with a dictionary of lockable objects wherein we traditionally would have + * used the synchronized keyword. + */ +public final class KeyAwareAutoCloseableLock { + private final Map internalMap = new ConcurrentHashMap<>(); + + public AutoReleasable lockForObject(Object key) { + AutoCloseableLock internal = + internalMap.computeIfAbsent(Objects.requireNonNull(key), k -> new AutoCloseableLock()); + return new AutoReleasable(internal.lock(), () -> internalMap.remove(key)); + } + + public static class AutoReleasable implements AutoCloseable { + + private final AutoCloseableLock lock; + private final Runnable cleanupAction; + + AutoReleasable(AutoCloseableLock lock, Runnable cleanupAction) { + this.lock = Objects.requireNonNull(lock); + this.cleanupAction = Objects.requireNonNull(cleanupAction); + } + + @Override + public void close() { + lock.close(); + cleanupAction.run(); + } + } +} diff --git a/testng-core/src/main/java/org/testng/SkipException.java b/testng-core/src/main/java/org/testng/SkipException.java index c9ffc42f29..1e6fafbc8d 100644 --- a/testng-core/src/main/java/org/testng/SkipException.java +++ b/testng-core/src/main/java/org/testng/SkipException.java @@ -1,5 +1,7 @@ package org.testng; +import org.testng.internal.AutoCloseableLock; + /** * The root exception for special skip handling. In case a @Test or @Configuration throws this * exception the method will be considered a skip or a failure according to the return of {@link @@ -34,6 +36,8 @@ public boolean isSkip() { return true; } + private final AutoCloseableLock internalLock = new AutoCloseableLock(); + /** * Subclasses may use this method to reduce the printed stack trace. This method keeps only the * last frame. Important: after calling this method the preserved internally and can be @@ -41,7 +45,7 @@ public boolean isSkip() { */ protected void reduceStackTrace() { if (!m_stackReduced) { - synchronized (this) { + try (AutoCloseableLock ignore = internalLock.lock()) { StackTraceElement[] newStack = new StackTraceElement[1]; StackTraceElement[] originalStack = getStackTrace(); if (originalStack.length > 0) { @@ -60,7 +64,7 @@ protected void reduceStackTrace() { */ protected void restoreStackTrace() { if (m_stackReduced && null != m_stackTrace) { - synchronized (this) { + try (AutoCloseableLock ignore = internalLock.lock()) { setStackTrace(m_stackTrace); m_stackReduced = false; } diff --git a/testng-core/src/main/java/org/testng/SuiteRunner.java b/testng-core/src/main/java/org/testng/SuiteRunner.java index bb6baffe15..41244e821f 100644 --- a/testng-core/src/main/java/org/testng/SuiteRunner.java +++ b/testng-core/src/main/java/org/testng/SuiteRunner.java @@ -415,12 +415,14 @@ private void runSequentially() { } } + private final AutoCloseableLock suiteResultsLock = new AutoCloseableLock(); + private void runTest(TestRunner tr) { visualisers.forEach(tr::addListener); tr.run(); ISuiteResult sr = new SuiteResult(xmlSuite, tr); - synchronized (suiteResults) { + try (AutoCloseableLock ignore = suiteResultsLock.lock()) { suiteResults.put(tr.getName(), sr); } } diff --git a/testng-core/src/main/java/org/testng/internal/ConfigurationGroupMethods.java b/testng-core/src/main/java/org/testng/internal/ConfigurationGroupMethods.java index 4c396eea1d..0a321571f4 100644 --- a/testng-core/src/main/java/org/testng/internal/ConfigurationGroupMethods.java +++ b/testng-core/src/main/java/org/testng/internal/ConfigurationGroupMethods.java @@ -28,8 +28,11 @@ public class ConfigurationGroupMethods { /** The list of beforeGroups methods keyed by the name of the group */ private final Map> m_beforeGroupsMethods; + private final AutoCloseableLock beforeGroups = new AutoCloseableLock(); private final Map beforeGroupsThatHaveAlreadyRun = new ConcurrentHashMap<>(); + + private final AutoCloseableLock afterGroups = new AutoCloseableLock(); private final Set afterGroupsThatHaveAlreadyRun = Collections.newSetFromMap(new ConcurrentHashMap<>()); @@ -64,7 +67,7 @@ public List getBeforeGroupMethodsForGroup(String[] groups) { return Collections.emptyList(); } - synchronized (beforeGroupsThatHaveAlreadyRun) { + try (AutoCloseableLock ignore = beforeGroups.lock()) { return Arrays.stream(groups) .map(t -> retrieve(beforeGroupsThatHaveAlreadyRun, m_beforeGroupsMethods, t)) .filter(Objects::nonNull) @@ -79,8 +82,7 @@ public List getAfterGroupMethods(ITestNGMethod testMethod) { } Set methodGroups = new HashSet<>(Arrays.asList(testMethod.getGroups())); - - synchronized (afterGroupsThatHaveAlreadyRun) { + try (AutoCloseableLock ignore = afterGroups.lock()) { if (m_afterGroupsMap == null) { m_afterGroupsMap = initializeAfterGroupsMap(); } @@ -151,7 +153,7 @@ private Map> initializeAfterGroupsMap() { } } - synchronized (afterGroupsThatHaveAlreadyRun) { + try (AutoCloseableLock ignore = afterGroups.lock()) { afterGroupsThatHaveAlreadyRun.clear(); } diff --git a/testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java b/testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java index f57c31791f..30e9d0ec4c 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java @@ -28,6 +28,7 @@ import org.testng.annotations.IConfigurationAnnotation; import org.testng.collections.Maps; import org.testng.collections.Sets; +import org.testng.internal.AutoCloseableLock; import org.testng.internal.ClassHelper; import org.testng.internal.ConfigurationMethod; import org.testng.internal.ConstructorOrMethod; @@ -556,8 +557,10 @@ private void setMethodInvocationFailure(ITestNGMethod method, Object instance) { instances.add(TestNgMethodUtils.getMethodInvocationToken(method, instance)); } + private final AutoCloseableLock internalLock = new AutoCloseableLock(); + private void setClassInvocationFailure(Class clazz, Object instance) { - synchronized (m_classInvocationResults) { + try (AutoCloseableLock ignore = internalLock.lock()) { Set instances = m_classInvocationResults.computeIfAbsent(clazz, k -> Sets.newHashSet()); Object objectToAdd = instance == null ? NULL_OBJECT : instance; diff --git a/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java b/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java index bcf0a4ab90..2a04302d78 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java @@ -848,50 +848,58 @@ public ITestResult registerSkippedTestResult( return result; } - private static final Object LOCK = new Object(); + private static final AutoCloseableLock LOCK = new AutoCloseableLock(); private StatusHolder considerExceptions( ITestNGMethod tm, ITestResult testResult, ExpectedExceptionsHolder exceptionsHolder, FailureContext failure) { - synchronized (LOCK) { - StatusHolder holder = new StatusHolder(); - int status = testResult.getStatus(); - holder.handled = false; - - Throwable ite = testResult.getThrowable(); - if (status == ITestResult.FAILURE && ite != null) { - - // Invocation caused an exception, see if the method was annotated with @ExpectedException - if (exceptionsHolder != null) { - if (exceptionsHolder.isExpectedException(ite)) { - testResult.setStatus(ITestResult.SUCCESS); - status = ITestResult.SUCCESS; + try (AutoCloseableLock ignore = LOCK.lock()) { + return considerExceptionsInternal(tm, testResult, exceptionsHolder, failure); + } + } + + private StatusHolder considerExceptionsInternal( + ITestNGMethod tm, + ITestResult testResult, + ExpectedExceptionsHolder exceptionsHolder, + FailureContext failure) { + StatusHolder holder = new StatusHolder(); + int status = testResult.getStatus(); + holder.handled = false; + + Throwable ite = testResult.getThrowable(); + if (status == ITestResult.FAILURE && ite != null) { + + // Invocation caused an exception, see if the method was annotated with @ExpectedException + if (exceptionsHolder != null) { + if (exceptionsHolder.isExpectedException(ite)) { + testResult.setStatus(ITestResult.SUCCESS); + status = ITestResult.SUCCESS; + } else { + if (isSkipExceptionAndSkip(ite)) { + status = ITestResult.SKIP; } else { - if (isSkipExceptionAndSkip(ite)) { - status = ITestResult.SKIP; - } else { - testResult.setThrowable(exceptionsHolder.wrongException(ite)); - status = ITestResult.FAILURE; - } + testResult.setThrowable(exceptionsHolder.wrongException(ite)); + status = ITestResult.FAILURE; } - } else { - handleException(ite, tm, testResult, failure.count.getAndIncrement()); - holder.handled = true; - status = testResult.getStatus(); - } - } else if (status != ITestResult.SKIP && exceptionsHolder != null) { - TestException exception = exceptionsHolder.noException(tm); - if (exception != null) { - testResult.setThrowable(exception); - status = ITestResult.FAILURE; } + } else { + handleException(ite, tm, testResult, failure.count.getAndIncrement()); + holder.handled = true; + status = testResult.getStatus(); + } + } else if (status != ITestResult.SKIP && exceptionsHolder != null) { + TestException exception = exceptionsHolder.noException(tm); + if (exception != null) { + testResult.setThrowable(exception); + status = ITestResult.FAILURE; } - holder.originalStatus = testResult.getStatus(); - holder.status = status; - return holder; } + holder.originalStatus = testResult.getStatus(); + holder.status = status; + return holder; } private static void updateStatusHolderAccordingToTestResult( diff --git a/testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java b/testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java index 248022fd30..f15806c90f 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java @@ -102,6 +102,8 @@ public String toString() { return result.toString(); } + private static final KeyAwareAutoCloseableLock lock = new KeyAwareAutoCloseableLock(); + /** * Run all the ITestNGMethods passed in through the constructor. * @@ -119,8 +121,9 @@ && doesTaskHavePreRequisites() for (IMethodInstance testMethodInstance : m_methodInstances) { ITestNGMethod testMethod = testMethodInstance.getMethod(); + Object key = testMethod.getInstance(); if (canInvokeBeforeClassMethods()) { - synchronized (testMethod.getInstance()) { + try (KeyAwareAutoCloseableLock.AutoReleasable ignore = lock.lockForObject(key)) { invokeBeforeClassMethods(testMethod.getTestClass(), testMethodInstance); } } @@ -129,7 +132,7 @@ && doesTaskHavePreRequisites() try { invokeTestMethods(testMethod, testMethodInstance.getInstance()); } finally { - synchronized (testMethod.getInstance()) { + try (KeyAwareAutoCloseableLock.AutoReleasable ignore = lock.lockForObject(key)) { invokeAfterClassMethods(testMethod.getTestClass(), testMethodInstance); } } diff --git a/testng-core/src/main/java/org/testng/internal/thread/graph/GraphOrchestrator.java b/testng-core/src/main/java/org/testng/internal/thread/graph/GraphOrchestrator.java index b472bb103a..3ddca247d6 100644 --- a/testng-core/src/main/java/org/testng/internal/thread/graph/GraphOrchestrator.java +++ b/testng-core/src/main/java/org/testng/internal/thread/graph/GraphOrchestrator.java @@ -6,6 +6,7 @@ import java.util.concurrent.*; import org.testng.IDynamicGraph; import org.testng.collections.Maps; +import org.testng.internal.AutoCloseableLock; import org.testng.internal.RuntimeBehavior; import org.testng.log4testng.Logger; import org.testng.thread.IThreadWorkerFactory; @@ -23,6 +24,8 @@ public class GraphOrchestrator { private final Comparator comparator; private final IThreadWorkerFactory factory; + private final AutoCloseableLock internalLock = new AutoCloseableLock(); + public GraphOrchestrator( ExecutorService service, IThreadWorkerFactory factory, @@ -35,7 +38,7 @@ public GraphOrchestrator( } public void run() { - synchronized (graph) { + try (AutoCloseableLock ignore = internalLock.lock()) { List freeNodes = graph.getFreeNodes(); if (comparator != null) { freeNodes.sort(comparator); @@ -71,7 +74,7 @@ private void mapNodeToParent(List freeNodes) { } private void afterExecute(IWorker r, Throwable t) { - synchronized (graph) { + try (AutoCloseableLock ignore = internalLock.lock()) { setStatus(r, computeStatus(r)); if (graph.getNodeCount() == graph.getNodeCountWithStatus(IDynamicGraph.Status.FINISHED)) { service.shutdown(); @@ -112,7 +115,7 @@ private IDynamicGraph.Status computeStatus(IWorker worker) { } private void setStatus(IWorker worker, IDynamicGraph.Status status) { - synchronized (graph) { + try (AutoCloseableLock ignore = internalLock.lock()) { for (T m : worker.getTasks()) { graph.setStatus(m, status); } diff --git a/testng-core/src/main/java/org/testng/internal/thread/graph/GraphThreadPoolExecutor.java b/testng-core/src/main/java/org/testng/internal/thread/graph/GraphThreadPoolExecutor.java deleted file mode 100644 index 447d47b1e7..0000000000 --- a/testng-core/src/main/java/org/testng/internal/thread/graph/GraphThreadPoolExecutor.java +++ /dev/null @@ -1,171 +0,0 @@ -package org.testng.internal.thread.graph; - -import java.util.Comparator; -import java.util.List; -import java.util.Map; -import java.util.concurrent.BlockingQueue; -import java.util.concurrent.ThreadPoolExecutor; -import java.util.concurrent.TimeUnit; -import org.testng.IDynamicGraph; -import org.testng.IDynamicGraph.Status; -import org.testng.TestNGException; -import org.testng.collections.Maps; -import org.testng.internal.RuntimeBehavior; -import org.testng.internal.thread.TestNGThreadFactory; -import org.testng.log4testng.Logger; -import org.testng.thread.ITestNGThreadPoolExecutor; -import org.testng.thread.IThreadWorkerFactory; -import org.testng.thread.IWorker; - -/** - * An Executor that launches tasks per batches. It takes a {@code DynamicGraph} of tasks to be run - * and a {@code IThreadWorkerFactory} to initialize/create {@code Runnable} wrappers around those - * tasks - * - * @deprecated - This implementation stands deprecated as of TestNG v7.9.0. There are - * no alternatives for this implementation. - */ -@Deprecated -public class GraphThreadPoolExecutor extends ThreadPoolExecutor - implements ITestNGThreadPoolExecutor { - - private final IDynamicGraph m_graph; - private final IThreadWorkerFactory m_factory; - private final Map> mapping = Maps.newConcurrentMap(); - private final Map upstream = Maps.newConcurrentMap(); - private final Comparator m_comparator; - - public GraphThreadPoolExecutor( - String name, - IDynamicGraph graph, - IThreadWorkerFactory factory, - int corePoolSize, - int maximumPoolSize, - long keepAliveTime, - TimeUnit unit, - BlockingQueue workQueue, - Comparator comparator) { - super( - corePoolSize, - maximumPoolSize, - keepAliveTime, - unit, - workQueue, - new TestNGThreadFactory(name)); - m_graph = graph; - m_factory = factory; - m_comparator = comparator; - - if (m_graph.getFreeNodes().isEmpty()) { - throw new TestNGException("The graph of methods contains a cycle:" + graph); - } - } - - public void run() { - synchronized (m_graph) { - List freeNodes = m_graph.getFreeNodes(); - if (m_comparator != null) { - freeNodes.sort(m_comparator); - } - runNodes(freeNodes); - } - } - - /** Create one worker per node and execute them. */ - private void runNodes(List freeNodes) { - List> workers = m_factory.createWorkers(freeNodes); - mapNodeToWorker(workers, freeNodes); - - for (int ix = 0; ix < workers.size(); ix++) { - IWorker worker = workers.get(ix); - mapNodeToParent(freeNodes); - setStatus(worker, Status.RUNNING); - try { - execute(worker); - } catch (Exception ex) { - Logger.getLogger(GraphThreadPoolExecutor.class).error(ex.getMessage(), ex); - } - } - } - - @Override - @SuppressWarnings("unchecked") - public void afterExecute(Runnable r, Throwable t) { - synchronized (m_graph) { - setStatus((IWorker) r, computeStatus(r)); - if (m_graph.getNodeCount() == m_graph.getNodeCountWithStatus(Status.FINISHED)) { - shutdown(); - } else { - List freeNodes = m_graph.getFreeNodes(); - if (m_comparator != null) { - freeNodes.sort(m_comparator); - } - handleThreadAffinity(freeNodes); - runNodes(freeNodes); - } - } - } - - private void setStatus(IWorker worker, IDynamicGraph.Status status) { - synchronized (m_graph) { - for (T m : worker.getTasks()) { - m_graph.setStatus(m, status); - } - } - } - - @SuppressWarnings("unchecked") - private IDynamicGraph.Status computeStatus(Runnable r) { - IWorker worker = (IWorker) r; - Status status = Status.FINISHED; - if (RuntimeBehavior.enforceThreadAffinity() && !worker.completed()) { - status = Status.READY; - } - return status; - } - - private void mapNodeToWorker(List> runnables, List freeNodes) { - if (!RuntimeBehavior.enforceThreadAffinity()) { - return; - } - for (IWorker runnable : runnables) { - for (T freeNode : freeNodes) { - IWorker w = mapping.get(freeNode); - if (w != null) { - long current = w.getThreadIdToRunOn(); - runnable.setThreadIdToRunOn(current); - } - if (runnable.toString().contains(freeNode.toString())) { - mapping.put(freeNode, runnable); - } - } - } - } - - private void mapNodeToParent(List freeNodes) { - if (!RuntimeBehavior.enforceThreadAffinity()) { - return; - } - for (T freeNode : freeNodes) { - List nodes = m_graph.getDependenciesFor(freeNode); - nodes.forEach(eachNode -> upstream.put(eachNode, freeNode)); - } - } - - private void handleThreadAffinity(List freeNodes) { - if (!RuntimeBehavior.enforceThreadAffinity()) { - return; - } - for (T node : freeNodes) { - T upstreamNode = upstream.get(node); - if (upstreamNode == null) { - continue; - } - IWorker w = mapping.get(upstreamNode); - if (w != null) { - long threadId = w.getCurrentThreadId(); - mapping.put(node, new PhoneyWorker<>(threadId)); - } - } - } -} diff --git a/testng-core/src/main/java/org/testng/reporters/JUnitXMLReporter.java b/testng-core/src/main/java/org/testng/reporters/JUnitXMLReporter.java index c11d300555..c479ea2859 100644 --- a/testng-core/src/main/java/org/testng/reporters/JUnitXMLReporter.java +++ b/testng-core/src/main/java/org/testng/reporters/JUnitXMLReporter.java @@ -14,6 +14,7 @@ import org.testng.ITestResult; import org.testng.collections.Maps; import org.testng.collections.Sets; +import org.testng.internal.AutoCloseableLock; import org.testng.internal.IResultListener2; import org.testng.internal.Utils; import org.testng.util.TimeUtils; @@ -146,21 +147,26 @@ static String formattedTime() { System.currentTimeMillis(), XMLReporterConfig.FMT_DEFAULT); } - private synchronized void createElementFromTestResults( + private final AutoCloseableLock lock = new AutoCloseableLock(); + + private void createElementFromTestResults( XMLStringBuffer document, Collection results) { - for (ITestResult tr : results) { - createElement(document, tr); + try (AutoCloseableLock ignore = lock.lock()) { + for (ITestResult tr : results) { + createElement(document, tr); + } } } - private synchronized void createElementFromIgnoredTests( - XMLStringBuffer doc, ITestContext context) { - Collection methods = context.getExcludedMethods(); - for (ITestNGMethod method : methods) { - Properties properties = getPropertiesFor(method, 0); - doc.push(XMLConstants.TESTCASE, properties); - doc.addEmptyElement(XMLConstants.ATTR_IGNORED); - doc.pop(); + private void createElementFromIgnoredTests(XMLStringBuffer doc, ITestContext context) { + try (AutoCloseableLock ignore = lock.lock()) { + Collection methods = context.getExcludedMethods(); + for (ITestNGMethod method : methods) { + Properties properties = getPropertiesFor(method, 0); + doc.push(XMLConstants.TESTCASE, properties); + doc.addEmptyElement(XMLConstants.ATTR_IGNORED); + doc.pop(); + } } } diff --git a/testng-core/src/main/java/org/testng/xml/XMLParser.java b/testng-core/src/main/java/org/testng/xml/XMLParser.java index 0ac044f09b..2386d403c5 100644 --- a/testng-core/src/main/java/org/testng/xml/XMLParser.java +++ b/testng-core/src/main/java/org/testng/xml/XMLParser.java @@ -7,6 +7,7 @@ import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; import org.testng.TestNGException; +import org.testng.internal.AutoCloseableLock; import org.testng.log4testng.Logger; import org.xml.sax.SAXException; import org.xml.sax.helpers.DefaultHandler; @@ -32,8 +33,10 @@ public abstract class XMLParser implements IFileParser { m_saxParser = parser; } + private static final AutoCloseableLock lock = new AutoCloseableLock(); + public void parse(InputStream is, DefaultHandler dh) throws SAXException, IOException { - synchronized (m_saxParser) { + try (AutoCloseableLock ignore = lock.lock()) { m_saxParser.parse(is, dh); } } diff --git a/testng-core/src/test/java/test/configuration/ConfigurationGroupBothSampleTest.java b/testng-core/src/test/java/test/configuration/ConfigurationGroupBothSampleTest.java index 3b890fb5c5..b4d04123f7 100644 --- a/testng-core/src/test/java/test/configuration/ConfigurationGroupBothSampleTest.java +++ b/testng-core/src/test/java/test/configuration/ConfigurationGroupBothSampleTest.java @@ -7,12 +7,17 @@ import org.testng.annotations.BeforeGroups; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; +import org.testng.internal.AutoCloseableLock; public class ConfigurationGroupBothSampleTest { static List m_list = Collections.synchronizedList(new ArrayList<>()); - private static synchronized void addToList(Integer n) { - m_list.add(n); + private static final AutoCloseableLock lock = new AutoCloseableLock(); + + private static void addToList(Integer n) { + try (AutoCloseableLock ignore = lock.lock()) { + m_list.add(n); + } } @BeforeGroups( diff --git a/testng-core/src/test/java/test/inheritance/issue2489/tests/BaseClassA.java b/testng-core/src/test/java/test/inheritance/issue2489/tests/BaseClassA.java index 151df125ae..ed9db06d31 100644 --- a/testng-core/src/test/java/test/inheritance/issue2489/tests/BaseClassA.java +++ b/testng-core/src/test/java/test/inheritance/issue2489/tests/BaseClassA.java @@ -6,9 +6,11 @@ import org.testng.annotations.BeforeClass; import org.testng.annotations.BeforeSuite; import org.testng.collections.Lists; +import org.testng.internal.AutoCloseableLock; public class BaseClassA { public static final List logs = Lists.newArrayList(); + private final AutoCloseableLock lock = new AutoCloseableLock(); @BeforeSuite(groups = "a") public void beforeSuite() { @@ -30,7 +32,9 @@ public void afterAllClasses() { print("afterClasses_BaseClass_A"); } - protected synchronized void print(String message) { - logs.add(message); + protected void print(String message) { + try (AutoCloseableLock ignore = lock.lock()) { + logs.add(message); + } } } diff --git a/testng-core/src/test/java/test/junitreports/TestClassContainerForGithubIssue1265.java b/testng-core/src/test/java/test/junitreports/TestClassContainerForGithubIssue1265.java index a930fab522..becce0a4d8 100644 --- a/testng-core/src/test/java/test/junitreports/TestClassContainerForGithubIssue1265.java +++ b/testng-core/src/test/java/test/junitreports/TestClassContainerForGithubIssue1265.java @@ -10,10 +10,10 @@ public class TestClassContainerForGithubIssue1265 { public abstract static class ParentTest { @BeforeSuite - public synchronized void startEverything() throws Exception {} + public void startEverything() {} @AfterSuite - public synchronized void shutdownContainer() throws Exception {} + public void shutdownContainer() {} } public static class FirstTest extends ParentTest { diff --git a/testng-core/src/test/java/test/listeners/factory/ExampleListener.java b/testng-core/src/test/java/test/listeners/factory/ExampleListener.java index bca6c6a3c2..fad89138c4 100644 --- a/testng-core/src/test/java/test/listeners/factory/ExampleListener.java +++ b/testng-core/src/test/java/test/listeners/factory/ExampleListener.java @@ -1,18 +1,23 @@ package test.listeners.factory; +import java.util.concurrent.atomic.AtomicBoolean; import org.testng.IExecutionListener; public class ExampleListener implements IExecutionListener { public static ExampleListener instance; + private static final AtomicBoolean once = new AtomicBoolean(false); + public ExampleListener() { setInstance(this); } - private static synchronized void setInstance(ExampleListener instance) { - if (ExampleListener.instance == null) { - ExampleListener.instance = instance; + private static void setInstance(ExampleListener instance) { + if (once.compareAndSet(false, true)) { + if (ExampleListener.instance == null) { + ExampleListener.instance = instance; + } } } } diff --git a/testng-core/src/test/java/test/listeners/issue2043/listeners/FailFastListener.java b/testng-core/src/test/java/test/listeners/issue2043/listeners/FailFastListener.java index 23fa2ba9ff..c659e30adc 100644 --- a/testng-core/src/test/java/test/listeners/issue2043/listeners/FailFastListener.java +++ b/testng-core/src/test/java/test/listeners/issue2043/listeners/FailFastListener.java @@ -1,6 +1,7 @@ package test.listeners.issue2043.listeners; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import org.testng.IConfigurationListener; import org.testng.IDataProviderListener; import org.testng.IDataProviderMethod; @@ -9,14 +10,13 @@ import org.testng.ITestContext; import org.testng.ITestNGMethod; import org.testng.ITestResult; -import org.testng.collections.Sets; public class FailFastListener implements IInvokedMethodListener, IConfigurationListener, IDataProviderListener { - public static final Set msgs = Sets.newHashSet(); + public static final Set msgs = ConcurrentHashMap.newKeySet(); @Override - public synchronized void afterInvocation(IInvokedMethod method, ITestResult testResult) { + public void afterInvocation(IInvokedMethod method, ITestResult testResult) { msgs.add(getClass().getSimpleName() + ":afterInvocation"); } diff --git a/testng-core/src/test/java/test/retryAnalyzer/issue2798/HashCodeAwareRetryAnalyzer.java b/testng-core/src/test/java/test/retryAnalyzer/issue2798/HashCodeAwareRetryAnalyzer.java index 60a2f41249..e82a22ce69 100644 --- a/testng-core/src/test/java/test/retryAnalyzer/issue2798/HashCodeAwareRetryAnalyzer.java +++ b/testng-core/src/test/java/test/retryAnalyzer/issue2798/HashCodeAwareRetryAnalyzer.java @@ -4,17 +4,22 @@ import java.util.List; import org.testng.IRetryAnalyzer; import org.testng.ITestResult; +import org.testng.internal.AutoCloseableLock; public class HashCodeAwareRetryAnalyzer implements IRetryAnalyzer { public static final List hashCodes = new ArrayList<>(); + private static final AutoCloseableLock lock = new AutoCloseableLock(); + int cnt = 0; static final int threshold = 2; @Override - public synchronized boolean retry(ITestResult result) { - hashCodes.add(this.hashCode()); - return cnt++ < threshold; + public boolean retry(ITestResult result) { + try (AutoCloseableLock ignore = lock.lock()) { + hashCodes.add(this.hashCode()); + return cnt++ < threshold; + } } } diff --git a/testng-core/src/test/java/test/testng1232/TestListenerFor1232.java b/testng-core/src/test/java/test/testng1232/TestListenerFor1232.java index e8d481c654..0357ae19d3 100644 --- a/testng-core/src/test/java/test/testng1232/TestListenerFor1232.java +++ b/testng-core/src/test/java/test/testng1232/TestListenerFor1232.java @@ -5,13 +5,17 @@ import java.util.concurrent.atomic.AtomicInteger; import org.testng.*; import org.testng.collections.Maps; +import org.testng.internal.AutoCloseableLock; import org.testng.xml.XmlSuite; public class TestListenerFor1232 extends ListenerTemplate { static Map counters = Maps.newHashMap(); + private static final AutoCloseableLock lock = new AutoCloseableLock(); - static synchronized void resetCounters() { - counters = Maps.newHashMap(); + static void resetCounters() { + try (AutoCloseableLock ignore = lock.lock()) { + counters = Maps.newHashMap(); + } } @Override diff --git a/testng-core/src/test/java/test/thread/BaseThreadTest.java b/testng-core/src/test/java/test/thread/BaseThreadTest.java index 96acad8fc4..5947ca0aea 100644 --- a/testng-core/src/test/java/test/thread/BaseThreadTest.java +++ b/testng-core/src/test/java/test/thread/BaseThreadTest.java @@ -7,6 +7,7 @@ import org.testng.collections.Lists; import org.testng.collections.Maps; import org.testng.collections.Sets; +import org.testng.internal.AutoCloseableLock; import test.SimpleBaseTest; public class BaseThreadTest extends SimpleBaseTest { @@ -14,6 +15,10 @@ public class BaseThreadTest extends SimpleBaseTest { private static Map m_suitesMap; private static List m_strings; + private static final AutoCloseableLock stringsLock = new AutoCloseableLock(); + private static final AutoCloseableLock threadIdsLock = new AutoCloseableLock(); + private static final AutoCloseableLock suiteMapLock = new AutoCloseableLock(); + static void initThreadLog() { m_threadIds = Sets.newHashSet(); m_suitesMap = Maps.newHashMap(); @@ -21,7 +26,7 @@ static void initThreadLog() { } protected void logString(String s) { - synchronized (m_strings) { + try (AutoCloseableLock ignore = stringsLock.lock()) { log("BaseThreadTest", "Logging string:" + s); m_strings.add(s); } @@ -36,20 +41,20 @@ protected void logCurrentThread() { } protected void logThread(long threadId) { - synchronized (m_threadIds) { + try (AutoCloseableLock ignore = threadIdsLock.lock()) { log("BaseThreadTest", "Logging thread:" + threadId); m_threadIds.add(threadId); } } protected void logSuite(String suiteName, long time) { - synchronized (m_suitesMap) { + try (AutoCloseableLock ignore = suiteMapLock.lock()) { m_suitesMap.put(suiteName, time); } } static int getThreadCount() { - synchronized (m_threadIds) { + try (AutoCloseableLock ignore = threadIdsLock.lock()) { return m_threadIds.size(); } } diff --git a/testng-core/src/test/java/test/thread/MultiThreadedDependentSampleTest.java b/testng-core/src/test/java/test/thread/MultiThreadedDependentSampleTest.java index 2b31fba9a9..09bee6cff8 100644 --- a/testng-core/src/test/java/test/thread/MultiThreadedDependentSampleTest.java +++ b/testng-core/src/test/java/test/thread/MultiThreadedDependentSampleTest.java @@ -7,6 +7,7 @@ import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; import org.testng.collections.Lists; +import org.testng.internal.AutoCloseableLock; public class MultiThreadedDependentSampleTest { @@ -108,14 +109,16 @@ private void logThread() { try { // With a lower value, tests fail sometimes Thread.sleep(40); - } catch (InterruptedException e) { + } catch (InterruptedException ignore) { } long id = Thread.currentThread().getId(); Helper.getMap(getClass().getName()).put(id, id); } + private static final AutoCloseableLock lock = new AutoCloseableLock(); + private void log(String string) { - synchronized (m_methods) { + try (AutoCloseableLock ignore = lock.lock()) { m_methods.add(string); } } diff --git a/testng-core/src/test/java/test/thread/MultiThreadedDependentTest.java b/testng-core/src/test/java/test/thread/MultiThreadedDependentTest.java index b686747205..bb23e38d32 100644 --- a/testng-core/src/test/java/test/thread/MultiThreadedDependentTest.java +++ b/testng-core/src/test/java/test/thread/MultiThreadedDependentTest.java @@ -8,6 +8,7 @@ import org.testng.annotations.Test; import org.testng.collections.Lists; import org.testng.collections.Maps; +import org.testng.internal.KeyAwareAutoCloseableLock; import org.testng.xml.XmlSuite; import test.SimpleBaseTest; @@ -70,7 +71,8 @@ private void test(int threadCount) { tng.setThreadCount(threadCount); tng.setParallel(XmlSuite.ParallelMode.METHODS); Map map = Helper.getMap(MultiThreadedDependentSampleTest.class.getName()); - synchronized (map) { + KeyAwareAutoCloseableLock lock = new KeyAwareAutoCloseableLock(); + try (KeyAwareAutoCloseableLock.AutoReleasable ignore = lock.lockForObject(map)) { tng.run(); Assert.assertTrue(map.size() > 1, "Map size:" + map.size() + " expected more than 1"); assertOrder(MultiThreadedDependentSampleTest.m_methods); From 9d0ba52064e53b65048ba62a23551c53b62b04bb Mon Sep 17 00:00:00 2001 From: Krishnan Mahadevan Date: Mon, 26 Feb 2024 18:17:07 +0530 Subject: [PATCH 2/4] Fixing review comments --- .../testng/internal/invokers/TestInvoker.java | 4 ++-- .../test/listeners/factory/ExampleListener.java | 16 +++++++--------- .../listeners/factory/TestNGFactoryTest.java | 2 +- .../test/thread/MultiThreadedDependentTest.java | 3 ++- 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java b/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java index 2a04302d78..308005e7f5 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java @@ -848,14 +848,14 @@ public ITestResult registerSkippedTestResult( return result; } - private static final AutoCloseableLock LOCK = new AutoCloseableLock(); + private static final AutoCloseableLock internalLock = new AutoCloseableLock(); private StatusHolder considerExceptions( ITestNGMethod tm, ITestResult testResult, ExpectedExceptionsHolder exceptionsHolder, FailureContext failure) { - try (AutoCloseableLock ignore = LOCK.lock()) { + try (AutoCloseableLock ignore = internalLock.lock()) { return considerExceptionsInternal(tm, testResult, exceptionsHolder, failure); } } diff --git a/testng-core/src/test/java/test/listeners/factory/ExampleListener.java b/testng-core/src/test/java/test/listeners/factory/ExampleListener.java index fad89138c4..4a532630a7 100644 --- a/testng-core/src/test/java/test/listeners/factory/ExampleListener.java +++ b/testng-core/src/test/java/test/listeners/factory/ExampleListener.java @@ -1,23 +1,21 @@ package test.listeners.factory; -import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; import org.testng.IExecutionListener; public class ExampleListener implements IExecutionListener { - public static ExampleListener instance; - - private static final AtomicBoolean once = new AtomicBoolean(false); + private static final AtomicReference instance = new AtomicReference<>(); public ExampleListener() { setInstance(this); } private static void setInstance(ExampleListener instance) { - if (once.compareAndSet(false, true)) { - if (ExampleListener.instance == null) { - ExampleListener.instance = instance; - } - } + ExampleListener.instance.compareAndSet(null, instance); + } + + public static ExampleListener getInstance() { + return instance.get(); } } diff --git a/testng-core/src/test/java/test/listeners/factory/TestNGFactoryTest.java b/testng-core/src/test/java/test/listeners/factory/TestNGFactoryTest.java index 87d4398b4c..e3d29b186e 100644 --- a/testng-core/src/test/java/test/listeners/factory/TestNGFactoryTest.java +++ b/testng-core/src/test/java/test/listeners/factory/TestNGFactoryTest.java @@ -23,7 +23,7 @@ public void testListenerFactoryViaConfigurationArg() { }; TestNG testng = TestNG.privateMain(args, null); assertThat(SampleTestFactory.instance).isNotNull(); - assertThat(ExampleListener.instance).isNotNull(); + assertThat(ExampleListener.getInstance()).isNotNull(); assertThat(testng.getStatus()).isZero(); } diff --git a/testng-core/src/test/java/test/thread/MultiThreadedDependentTest.java b/testng-core/src/test/java/test/thread/MultiThreadedDependentTest.java index bb23e38d32..361ec7fb1d 100644 --- a/testng-core/src/test/java/test/thread/MultiThreadedDependentTest.java +++ b/testng-core/src/test/java/test/thread/MultiThreadedDependentTest.java @@ -64,6 +64,8 @@ public void test3Threads() { test(3); } + private final KeyAwareAutoCloseableLock lock = new KeyAwareAutoCloseableLock(); + private void test(int threadCount) { Helper.reset(); MultiThreadedDependentSampleTest.m_methods = Lists.newArrayList(); @@ -71,7 +73,6 @@ private void test(int threadCount) { tng.setThreadCount(threadCount); tng.setParallel(XmlSuite.ParallelMode.METHODS); Map map = Helper.getMap(MultiThreadedDependentSampleTest.class.getName()); - KeyAwareAutoCloseableLock lock = new KeyAwareAutoCloseableLock(); try (KeyAwareAutoCloseableLock.AutoReleasable ignore = lock.lockForObject(map)) { tng.run(); Assert.assertTrue(map.size() > 1, "Map size:" + map.size() + " expected more than 1"); From cc23c7b38de591b112a9423e51297f39bea0aaf1 Mon Sep 17 00:00:00 2001 From: Krishnan Mahadevan Date: Tue, 27 Feb 2024 18:48:47 +0530 Subject: [PATCH 3/4] Adding a test for KeyAwareAutoCloseableLockTest --- .../testng/internal/AutoCloseableLock.java | 18 +++++++++++++ .../internal/KeyAwareAutoCloseableLock.java | 16 +++++++++++- .../internal/invokers/TestMethodWorker.java | 2 +- .../KeyAwareAutoCloseableLockTest.java | 26 +++++++++++++++++++ testng-core/src/test/resources/testng.xml | 1 + 5 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 testng-core/src/test/java/org/testng/internal/KeyAwareAutoCloseableLockTest.java diff --git a/testng-core-api/src/main/java/org/testng/internal/AutoCloseableLock.java b/testng-core-api/src/main/java/org/testng/internal/AutoCloseableLock.java index bb4e612246..213d6d04c2 100644 --- a/testng-core-api/src/main/java/org/testng/internal/AutoCloseableLock.java +++ b/testng-core-api/src/main/java/org/testng/internal/AutoCloseableLock.java @@ -1,6 +1,7 @@ package org.testng.internal; import java.io.Closeable; +import java.util.Objects; import java.util.concurrent.locks.ReentrantLock; /** @@ -16,8 +17,25 @@ public AutoCloseableLock lock() { return this; } + public boolean isHeldByCurrentThread() { + return internalLock.isHeldByCurrentThread(); + } + @Override public void close() { internalLock.unlock(); } + + @Override + public boolean equals(Object object) { + if (this == object) return true; + if (object == null || getClass() != object.getClass()) return false; + AutoCloseableLock that = (AutoCloseableLock) object; + return Objects.equals(internalLock, that.internalLock); + } + + @Override + public int hashCode() { + return Objects.hash(internalLock); + } } diff --git a/testng-core-api/src/main/java/org/testng/internal/KeyAwareAutoCloseableLock.java b/testng-core-api/src/main/java/org/testng/internal/KeyAwareAutoCloseableLock.java index 348d9d85cf..9b9df2b66c 100644 --- a/testng-core-api/src/main/java/org/testng/internal/KeyAwareAutoCloseableLock.java +++ b/testng-core-api/src/main/java/org/testng/internal/KeyAwareAutoCloseableLock.java @@ -25,7 +25,8 @@ public static class AutoReleasable implements AutoCloseable { AutoReleasable(AutoCloseableLock lock, Runnable cleanupAction) { this.lock = Objects.requireNonNull(lock); - this.cleanupAction = Objects.requireNonNull(cleanupAction); + this.cleanupAction = + this.lock.isHeldByCurrentThread() ? () -> {} : Objects.requireNonNull(cleanupAction); } @Override @@ -33,5 +34,18 @@ public void close() { lock.close(); cleanupAction.run(); } + + @Override + public boolean equals(Object object) { + if (this == object) return true; + if (object == null || getClass() != object.getClass()) return false; + AutoReleasable that = (AutoReleasable) object; + return Objects.equals(lock, that.lock); + } + + @Override + public int hashCode() { + return Objects.hash(lock); + } } } diff --git a/testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java b/testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java index f15806c90f..de61ff59db 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java @@ -121,7 +121,7 @@ && doesTaskHavePreRequisites() for (IMethodInstance testMethodInstance : m_methodInstances) { ITestNGMethod testMethod = testMethodInstance.getMethod(); - Object key = testMethod.getInstance(); + String key = Integer.toString(testMethod.getInstance().toString().hashCode()); if (canInvokeBeforeClassMethods()) { try (KeyAwareAutoCloseableLock.AutoReleasable ignore = lock.lockForObject(key)) { invokeBeforeClassMethods(testMethod.getTestClass(), testMethodInstance); diff --git a/testng-core/src/test/java/org/testng/internal/KeyAwareAutoCloseableLockTest.java b/testng-core/src/test/java/org/testng/internal/KeyAwareAutoCloseableLockTest.java new file mode 100644 index 0000000000..eacc54fb99 --- /dev/null +++ b/testng-core/src/test/java/org/testng/internal/KeyAwareAutoCloseableLockTest.java @@ -0,0 +1,26 @@ +package org.testng.internal; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.testng.annotations.Test; + +public class KeyAwareAutoCloseableLockTest { + + @Test + public void ensureLockIsReEntrant() { + String key = "TestNG"; + KeyAwareAutoCloseableLock lock = new KeyAwareAutoCloseableLock(); + KeyAwareAutoCloseableLock.AutoReleasable outer, inner1, inner2; + try (KeyAwareAutoCloseableLock.AutoReleasable ignore = lock.lockForObject(key)) { + outer = ignore; + try (KeyAwareAutoCloseableLock.AutoReleasable ignore1 = lock.lockForObject(key)) { + inner1 = ignore1; + } + try (KeyAwareAutoCloseableLock.AutoReleasable ignore2 = lock.lockForObject(key)) { + inner2 = ignore2; + } + } + assertThat(outer).isEqualTo(inner1); + assertThat(inner1).isEqualTo(inner2); + } +} diff --git a/testng-core/src/test/resources/testng.xml b/testng-core/src/test/resources/testng.xml index 5df51a1d55..0aba6f1bfa 100644 --- a/testng-core/src/test/resources/testng.xml +++ b/testng-core/src/test/resources/testng.xml @@ -86,6 +86,7 @@ + From 72e7598ac28944045c0bb23d71f7aea52057d1d2 Mon Sep 17 00:00:00 2001 From: Krishnan Mahadevan Date: Fri, 1 Mar 2024 12:48:39 +0530 Subject: [PATCH 4/4] Fallback to synchronised keyword --- .../org/testng/internal/invokers/TestMethodWorker.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java b/testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java index de61ff59db..248022fd30 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java @@ -102,8 +102,6 @@ public String toString() { return result.toString(); } - private static final KeyAwareAutoCloseableLock lock = new KeyAwareAutoCloseableLock(); - /** * Run all the ITestNGMethods passed in through the constructor. * @@ -121,9 +119,8 @@ && doesTaskHavePreRequisites() for (IMethodInstance testMethodInstance : m_methodInstances) { ITestNGMethod testMethod = testMethodInstance.getMethod(); - String key = Integer.toString(testMethod.getInstance().toString().hashCode()); if (canInvokeBeforeClassMethods()) { - try (KeyAwareAutoCloseableLock.AutoReleasable ignore = lock.lockForObject(key)) { + synchronized (testMethod.getInstance()) { invokeBeforeClassMethods(testMethod.getTestClass(), testMethodInstance); } } @@ -132,7 +129,7 @@ && doesTaskHavePreRequisites() try { invokeTestMethods(testMethod, testMethodInstance.getInstance()); } finally { - try (KeyAwareAutoCloseableLock.AutoReleasable ignore = lock.lockForObject(key)) { + synchronized (testMethod.getInstance()) { invokeAfterClassMethods(testMethod.getTestClass(), testMethodInstance); } }