Make InternalUnitDefinition.round thread-safe

vcell-math/src/main/java/cbit/vcell/units/InternalUnitDefinition.java

@@ -43,8 +43,6 @@
 class InternalUnitDefinition implements Matchable, Serializable {
 
     public static final String TBD_SYMBOL = "tbd";
-    private static final java.text.NumberFormat numberFormatForRounding =
-        new java.text.DecimalFormat("#0.0#E0#");
 
     //VC standard Unit definitions
     public static final InternalUnitDefinition UNIT_s;
@@ -110,8 +108,6 @@ class InternalUnitDefinition implements Matchable, Serializable {
     //private static ucar.units_vcell.PrefixDB prefixDB = null;
 
     static {
-        numberFormatForRounding.setMaximumFractionDigits(12);
-
         defs = new ArrayList<InternalUnitDefinition>();
 
         InternalUnitDefinition unit_s = null;
@@ -674,19 +670,20 @@ public InternalUnitDefinition raiseTo(RationalNumber exp) {
 
 
 /**
- * Insert the method's description here.
- * Creation date: (6/13/2004 3:03:36 PM)
- * @return double
- * @param value double
+ * Round a value to at most 12 significant digits.
+ *
+ * Implementation note: previously used a shared static DecimalFormat
+ * for round-trip format/parse. DecimalFormat is not thread-safe and the
+ * shared instance produced corrupted strings (e.g., "11E.11-83E-83")
+ * under concurrent access from background-thread unit arithmetic,
+ * causing NumberFormatException to escape the catch block. This
+ * implementation is stateless and locale-independent.
  */
 public static double round(double value) {
-	try {
-		double roundedValue = numberFormatForRounding.parse(numberFormatForRounding.format(value)).doubleValue();
-		return roundedValue;
-	}catch (java.text.ParseException e){
-		e.printStackTrace(System.out);
+	if (Double.isNaN(value) || Double.isInfinite(value)) {
 		return value;
 	}
+	return Double.parseDouble(String.format(java.util.Locale.US, "%.12e", value));
 }
 
 

vcell-math/src/test/java/cbit/vcell/units/InternalUnitDefinitionRoundTest.java

@@ -0,0 +1,89 @@
+package cbit.vcell.units;
+
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+@Tag("Fast")
+public class InternalUnitDefinitionRoundTest {
+
+    @Test
+    public void roundOfOrdinaryValuesIsIdempotentToTwelveDigits() {
+        double[] values = {
+                1.0, -1.0, 0.0, 1234.5678, -1234.5678,
+                1.234567890123456789, 1e-83, 1e83, Math.PI, Math.E,
+        };
+        for (double v : values) {
+            double r = InternalUnitDefinition.round(v);
+            // Twelve significant digits should be preserved exactly.
+            assertEquals(r, InternalUnitDefinition.round(r),
+                    "round must be idempotent for value " + v);
+            // The rounded value must match the original to 12 sig figs.
+            if (v != 0.0) {
+                double relErr = Math.abs((r - v) / v);
+                assertTrue(relErr < 1e-11,
+                        "relative error too large for " + v + ": " + relErr);
+            }
+        }
+    }
+
+    @Test
+    public void roundPreservesSpecialValues() {
+        assertTrue(Double.isNaN(InternalUnitDefinition.round(Double.NaN)));
+        assertEquals(Double.POSITIVE_INFINITY,
+                InternalUnitDefinition.round(Double.POSITIVE_INFINITY));
+        assertEquals(Double.NEGATIVE_INFINITY,
+                InternalUnitDefinition.round(Double.NEGATIVE_INFINITY));
+        assertEquals(0.0, InternalUnitDefinition.round(0.0));
+        // round(-0.0) returns -0.0 (sign preserved); both are mathematically zero.
+        assertEquals(0.0, Math.abs(InternalUnitDefinition.round(-0.0)));
+    }
+
+    @Test
+    public void roundIsThreadSafe() throws InterruptedException {
+        // Regression test: a previous implementation used a shared static
+        // DecimalFormat. Concurrent access produced strings like
+        // "11E.11-83E-83" (= "1E-83" formatted twice with digits
+        // interleaved), which then failed Double.parseDouble with
+        // NumberFormatException ("multiple points" or "For input string").
+        int threads = 16;
+        int iterationsPerThread = 5_000;
+        ExecutorService pool = Executors.newFixedThreadPool(threads);
+        AtomicInteger failures = new AtomicInteger(0);
+        AtomicInteger badResults = new AtomicInteger(0);
+        try {
+            for (int t = 0; t < threads; t++) {
+                final double base = 1.0 + t * 0.0001;
+                pool.submit(() -> {
+                    for (int i = 0; i < iterationsPerThread; i++) {
+                        double value = base * Math.pow(10, (i % 100) - 50);
+                        try {
+                            double r = InternalUnitDefinition.round(value);
+                            if (Double.isNaN(r) != Double.isNaN(value)) {
+                                badResults.incrementAndGet();
+                            }
+                        } catch (RuntimeException e) {
+                            failures.incrementAndGet();
+                        }
+                    }
+                });
+            }
+        } finally {
+            pool.shutdown();
+            assertTrue(pool.awaitTermination(30, TimeUnit.SECONDS),
+                    "thread pool did not terminate in time");
+        }
+        assertEquals(0, failures.get(),
+                "round() threw under concurrent access");
+        assertEquals(0, badResults.get(),
+                "round() returned wrong-typed result under concurrent access");
+    }
+}