Ensure thread safety for attribute access

CHANGES.txt

@@ -1,4 +1,5 @@
 Current
+Fixed: GITHUB-2991: Suite attributes map should be thread safe (Krishnan Mahadevan)
 Fixed: GITHUB-2974: Command line arguments -groups and -excludegroups override defined groups in a suite xml file (dr29bart)
 Fixed: GITHUB-2961: "Unexpected value: 16" error when multiple beforeMethod config methods with firstTimeOnly property run before a test (Krishnan Mahadevan)
 Fixed: GITHUB-2904: Add location of docs Github to readme and contributions page (Mohsin Sackeer)

testng-core/src/test/java/test/attributes/AttributeTest.java

@@ -4,10 +4,13 @@
 
 import java.util.Set;
 import org.testng.ITestContext;
+import org.testng.TestNG;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
+import test.SimpleBaseTest;
+import test.attributes.issue2991.TestClassSample;
 
-public class AttributeTest {
+public class AttributeTest extends SimpleBaseTest {
 
   @BeforeClass
   public void bc(ITestContext ctx) {
@@ -32,4 +35,11 @@ public void f2(ITestContext ctx) {
     assertThat(names).contains("test2");
     assertThat(ctx.getAttribute("test2")).isEqualTo("2");
   }
+
+  @Test(description = "GITHUB-2991")
+  public void ensureAttributeAccessIsThreadSafe() {
+    TestNG testng = create(TestClassSample.class);
+    testng.run();
+    assertThat(testng.getStatus()).isZero();
+  }
 }

testng-core/src/test/java/test/attributes/issue2991/TestClassSample.java

@@ -0,0 +1,45 @@
+package test.attributes.issue2991;
+
+import java.util.ConcurrentModificationException;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.assertj.core.api.SoftAssertions;
+import org.testng.IAttributes;
+import org.testng.ITestResult;
+import org.testng.Reporter;
+import org.testng.annotations.Test;
+
+public class TestClassSample {
+  private static final AtomicInteger counter = new AtomicInteger(1);
+
+  @Test(invocationCount = 100, threadPoolSize = 20)
+  public void sampleTest() {
+    String key = "test-" + counter.getAndIncrement();
+    ITestResult itr = Reporter.getCurrentTestResult();
+    SoftAssertions softly = new SoftAssertions();
+    softly
+        .assertThat(iterate(key, itr))
+        .withFailMessage("No exceptions at test result level")
+        .isTrue();
+    softly
+        .assertThat(iterate(key, itr.getTestContext()))
+        .withFailMessage("No exceptions at <test> level")
+        .isTrue();
+    softly
+        .assertThat(iterate(key, itr.getTestContext().getSuite()))
+        .withFailMessage("No exceptions at <suite> level")
+        .isTrue();
+    softly.assertAll();
+  }
+
+  private static boolean iterate(String key, IAttributes attributes) {
+    try {
+      for (String next : attributes.getAttributeNames()) {
+        Reporter.log(attributes.getAttribute(next).toString());
+      }
+      attributes.setAttribute(key, counter.get());
+      return true;
+    } catch (ConcurrentModificationException e) {
+      return false;
+    }
+  }
+}

testng-runner-api/src/main/java/org/testng/internal/Attributes.java

@@ -8,7 +8,7 @@
 /** Simple implementation of IAttributes. */
 public class Attributes implements IAttributes {
 
-  private final Map<String, Object> m_attributes = Maps.newHashMap();
+  private final Map<String, Object> m_attributes = Maps.newConcurrentMap();
 
   @Override
   public Object getAttribute(String name) {