Skip to content

Fix thread-safety (ConcurrentHashMap) and typos#130

Open
Dirzei wants to merge 1 commit into
EasyKayzey:masterfrom
Dirzei:fix/thread-safety-and-typos
Open

Fix thread-safety (ConcurrentHashMap) and typos#130
Dirzei wants to merge 1 commit into
EasyKayzey:masterfrom
Dirzei:fix/thread-safety-and-typos

Conversation

@Dirzei

@Dirzei Dirzei commented May 4, 2026

Copy link
Copy Markdown

CASprzak 0.3.0 — Issues Found and Solutions

Created — May 2026

================================================================================
ISSUE 1: Thread-Safety — GeneralFunction.derivatives (Race Condition)

FILE: src/core/functions/GeneralFunction.java
LINES: 57, 83-87

DESCRIPTION:
The derivative cache is implemented as a plain HashMap:

protected final Map<String, GeneralFunction> derivatives = new HashMap<>();

The method getSimplifiedDerivative() reads and writes this cache:

public GeneralFunction getSimplifiedDerivative(String varID) {
    if (Settings.cacheDerivatives && derivatives.containsKey(varID))
        return derivatives.get(varID);      // read
    GeneralFunction derivative = getDerivative(varID).simplify();
    if (Settings.cacheDerivatives)
        derivatives.put(varID, derivative); // WRITE -- RACE CONDITION
}

HashMap is not thread-safe. When multiple threads concurrently call
getSimplifiedDerivative() on the same GeneralFunction instance -- which
happens during parallel Jacobian construction in CMDSolver
(IntStream.parallel()) -- concurrent put() operations can cause:
- Data loss (entries overwritten or silently dropped)
- NullPointerException due to corrupted internal HashMap state
- Infinite loops caused by circular internal references
- Non-deterministic behavior -- difficult to reproduce and debug

recommended for CASprzak itself: Use ConcurrentHashMap

BEFORE:
import java.util.HashMap;
protected final Map<String, GeneralFunction> derivatives = new HashMap<>();

AFTER:
import java.util.concurrent.ConcurrentHashMap;
protected final Map<String, GeneralFunction> derivatives
= new ConcurrentHashMap<>();

ConcurrentHashMap is fully thread-safe for concurrent get() and put()
operations without explicit locking and with minimal performance overhead.
No further changes required.

================================================================================
ISSUE 2: Thread-Safety -- LegrendePolynomial.cache (static HashMap)

FILE: src/core/tools/functiongenerators/LegrendePolynomial.java
LINE: 22

DESCRIPTION:
The static polynomial cache is also implemented as a plain HashMap:

private static final Map<Integer, GeneralFunction> cache = new HashMap<>();

Static HashMap without synchronization -- same race condition as in
GeneralFunction.derivatives, but shared across all instances as a
class-level variable. Even more critical in a multithreaded context.

SOLUTION: Use ConcurrentHashMap

BEFORE:
import java.util.HashMap;
private static final Map<Integer, GeneralFunction> cache = new HashMap<>();

AFTER:
import java.util.concurrent.ConcurrentHashMap;
private static final Map<Integer, GeneralFunction> cache
= new ConcurrentHashMap<>();

================================================================================
ISSUE 3: Typo -- "Legrende" instead of "Legendre"

AFFECTED FILES:

  • src/core/tools/functiongenerators/LegrendePolynomial.java
    (class name, method names, Javadoc -- throughout)
  • src/core/config/Settings.java
    (lines 116, 118, 227: cacheLegrendePolynomials)
  • src/core/config/cas.properties
    (lines 53, 54: cacheLegrendePolynomials)

DESCRIPTION:
Legendre polynomials (named after Adrien-Marie Legendre, 1752-1833)
are consistently spelled "Legrende" throughout the codebase. This is
a typo that propagates through all related identifiers.

Correct spelling: Legendre (ending in 're', not 'e' in the middle)

NOTE ON CORRECTION:
The class name "LegrendePolynomial" and the Settings variable
"cacheLegrendePolynomials" are part of the public API. Renaming them
would be a breaking change. Recommended approach:

  • Fix the typo in all Javadoc comments immediately (done)
  • Add a note in the Javadoc acknowledging the historical typo (done)
  • Keep the identifier names unchanged for API compatibility
  • Optionally add a @deprecated alias pointing to a corrected class name
    in a future major version

================================================================================
ISSUE 4: Typo -- "\zeto" instead of "\zeta" in LatexReplacer

FILE: src/parsing/LatexReplacer.java
LINE: 36

DESCRIPTION:
The LaTeX-to-Unicode replacement map contains a typo in the lowercase
zeta entry. The LaTeX command pattern is registered as "\zeto" instead
of the correct "\zeta":

BEFORE (line 36):
  put(Pattern.compile("\\\\zeto" + "(?![\\w.'[^\\x00-\\x7F]])"), "z");

AFTER:
  put(Pattern.compile("\\\\zeta" + "(?![\\w.'[^\\x00-\\x7F]])"), "z");

IMPACT:
The LaTeX command \zeta (lowercase Greek letter zeta, Unicode z U+03B6)
is never matched. Any expression using \zeta as a variable name --
e.g. in fluid mechanics (damping ratio) or complex analysis -- would fail
silently: the replacement does not occur, and the literal string "\zeta"
is passed downstream instead of the Unicode character z.

Note that uppercase \Zeta (line 35) is correctly spelled and unaffected.

FIX: Replace "\\zeto" with "\\zeta" on line 36.
This is a one-character fix ('o' -> 'a') with no side effects.

================================================================================
SUMMARY

Issue 1 (Thread-Safety HashMap): CRITICAL -- Race condition under parallelism
Issue 2 (Thread-Safety static HashMap): CRITICAL -- Race condition under parallelism
Issue 3 (Typo "Legrende"): MINOR -- No functional impact
Issue 4 (Typo "\zeto" -> "\zeta"): MEDIUM -- \zeta LaTeX command never matched

For CMDSolver v2.13: Issue 1 resolved via Settings.cacheDerivatives=false.
Issues 2, 3, and 4 fixed directly in the CASprzak source files provided.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant