diff --git a/JavaToCSharp.Tests/ConcurrencyTests.cs b/JavaToCSharp.Tests/ConcurrencyTests.cs new file mode 100644 index 0000000..bc2beef --- /dev/null +++ b/JavaToCSharp.Tests/ConcurrencyTests.cs @@ -0,0 +1,82 @@ +using JavaToCSharp; + +namespace JavaToCSharp.Tests; + +/// +/// Regression tests for shared mutable state that previously corrupted conversions +/// when they ran concurrently. See the voidvoid return-type corruption caused by +/// holding its parse state in static fields. +/// +public class ConcurrencyTests +{ + /// + /// Hammers the type-name parser from many threads at once. Before the fix, the parser's + /// shared static StringBuilder caused threads to splice each other's output, producing + /// results like "voidvoid" or another thread's type name entirely. + /// + [Fact] + public void ConvertType_IsThreadSafe() + { + (string Java, string Expected)[] cases = + [ + ("void", "void"), + ("String", "string"), + ("Integer", "int"), + ("List", "IList"), + ("Map", "Dictionary"), + ("int[]", "int[]"), + ("List>", "IList>"), + ]; + + var failures = new System.Collections.Concurrent.ConcurrentBag(); + + Parallel.For(0, 2000, new ParallelOptions { MaxDegreeOfParallelism = 16 }, i => + { + var (java, expected) = cases[i % cases.Length]; + var actual = TypeHelper.ConvertType(java); + + if (actual != expected) + { + failures.Add($"ConvertType(\"{java}\") returned \"{actual}\", expected \"{expected}\""); + } + }); + + Assert.Empty(failures); + } + + /// + /// Runs full conversions in parallel. Before the fix these corrupted each other's output, + /// which surfaced as intermittent failures in unrelated test classes. + /// + [Fact] + public void Convert_IsThreadSafe() + { + const string java = """ + package com.example; + + public class Program { + public static void main(String[] args) { + System.out.println("Hello world!"); + } + + public List getNames(Map input) { + return null; + } + } + """; + + var expected = JavaToCSharpConverter.ConvertText(java, new JavaConversionOptions()); + Assert.NotNull(expected); + Assert.Contains("void Main", expected); + + var results = new System.Collections.Concurrent.ConcurrentBag(); + + Parallel.For(0, 200, new ParallelOptions { MaxDegreeOfParallelism = 16 }, _ => + { + results.Add(JavaToCSharpConverter.ConvertText(java, new JavaConversionOptions())); + }); + + // Every concurrent conversion must match the single-threaded result exactly. + Assert.All(results, r => Assert.Equal(expected, r)); + } +} diff --git a/JavaToCSharp/TypeHelper.cs b/JavaToCSharp/TypeHelper.cs index 7dcaafa..9b648a3 100644 --- a/JavaToCSharp/TypeHelper.cs +++ b/JavaToCSharp/TypeHelper.cs @@ -1,4 +1,5 @@ -using com.github.javaparser.ast.expr; +using System.Collections.Concurrent; +using com.github.javaparser.ast.expr; using com.github.javaparser.ast.type; using JavaToCSharp.Expressions; using Microsoft.CodeAnalysis; @@ -11,7 +12,10 @@ namespace JavaToCSharp; public static class TypeHelper { - private static readonly Dictionary _typeNameConversions = new() + // Mutated at conversion time via AddOrUpdateTypeNameConversions (e.g. to register interface + // renames when StartInterfaceNamesWithI is set) while other threads read it in ConvertType, + // so this must be a concurrent collection to keep parallel conversions safe. + private static readonly ConcurrentDictionary _typeNameConversions = new() { // Simple types ["boolean"] = "bool", diff --git a/JavaToCSharp/TypeNameParser.cs b/JavaToCSharp/TypeNameParser.cs index 6b297f6..a8c0d14 100644 --- a/JavaToCSharp/TypeNameParser.cs +++ b/JavaToCSharp/TypeNameParser.cs @@ -1,9 +1,16 @@ -using System.Text; +using System.Text; using System.Text.RegularExpressions; namespace JavaToCSharp; -public static partial class TypeNameParser +/// +/// Parses and translates Java type names into their C# equivalents. +/// +/// +/// All parse state is held in instance fields, and a fresh instance is created per +/// call, so concurrent conversions cannot corrupt each other. +/// +public sealed partial class TypeNameParser { private enum TokenType { @@ -22,11 +29,19 @@ private enum TokenType [GeneratedRegex(@"\w+|\[|\]|<|>|,|\?", RegexOptions.Compiled)] private static partial Regex TokenizePattern { get; } - private static (string, TokenType)[]? _tokens; - private static (string text, TokenType type) _token; - private static int _currentIndex; - private static readonly StringBuilder _sb = new(); - private static Func? _translate; + private readonly (string, TokenType)[] _tokens; + private readonly StringBuilder _sb = new(); + private readonly Func _translate; + private (string text, TokenType type) _token; + private int _currentIndex; + + private TypeNameParser(string typename, Func translateIdentifier) + { + _translate = translateIdentifier; + _tokens = Tokenize(typename); + _currentIndex = -1; + NextToken(); + } internal static string ParseTypeName(string typename, Func translateIdentifier) { @@ -36,16 +51,12 @@ internal static string ParseTypeName(string typename, Func trans // TypeName = identifier [ "<" TypeArgument { "," TypeArgument } ">" ] { "[" "]" }. // TypeArgument = [ "?" [ "extends" | "super" ] ] TypeName. - _translate = translateIdentifier; - _tokens = Tokenize(typename); - _currentIndex = -1; - NextToken(); - _sb.Clear(); + var parser = new TypeNameParser(typename, translateIdentifier); - if (TypeName() && _token.type is TokenType.EndOfString) + if (parser.TypeName() && parser._token.type is TokenType.EndOfString) { // Otherwise we have extra tokens. - return _sb.ToString(); + return parser._sb.ToString(); } return typename; @@ -75,13 +86,13 @@ private static (string, TokenType)[] Tokenize(string typeName) return tokens; } - private static void NextToken() + private void NextToken() { _currentIndex++; - _token = _tokens is not null && _currentIndex < _tokens.Length ? _tokens[_currentIndex] : default; + _token = _currentIndex < _tokens.Length ? _tokens[_currentIndex] : default; } - private static bool TypeName() + private bool TypeName() { // TypeName = identifier [ "<" TypeArgument { "," TypeArgument } ">" ] { "[" "]" }. if (_token.type is not TokenType.Identifier) @@ -89,7 +100,7 @@ private static bool TypeName() return false; } - _sb.Append(_translate?.Invoke(_token.text)); + _sb.Append(_translate(_token.text)); NextToken(); if (_token.type is TokenType.LeftAngleBracket) @@ -114,7 +125,7 @@ private static bool TypeName() return ArraySuffix(); } - private static bool ArraySuffix() + private bool ArraySuffix() { while (_token.type is TokenType.LeftSquareBracket) { @@ -128,7 +139,7 @@ private static bool ArraySuffix() return true; } - private static bool TypeArgument() + private bool TypeArgument() { // TypeArgument = [ "?" [ "extends" | "super" ] ] TypeName. if (_token.type is TokenType.QuestionMark)