Race Condition in ConstantExpressionHelper Causing Parsing Failures #904

Issue Summary

We encountered a race condition in ConstantExpressionHelper that can lead to parsing failures in ExpressionPromoter when _literals is cleaned up.
This issue is caused by SlidingCache, which automatically removes expired items, making Promote() unable to retrieve the required text representation of constants.

How This Issue Occurs

1. A thread calls CreateLiteral(value, text), storing:

   • _expressions.AddOrUpdate(value, constantExpression);
   • _literals.AddOrUpdate(constantExpression, text);

2. Another thread runs cache cleanup, removing _literals and _expressions due to the CleanupFrequency and TimeToLive properties in ConstantExpressionCacheConfig.

3. A third thread calls Promote(), which attempts to retrieve the text using:

   _constantExpressionHelper.TryGetText(ce, out var text);

   • Since _literals was cleaned, TryGetText() returns false, and Promote() fails to perform the type promotion.

4. This leads to an error when trying to compare a double with a decimal, causing:

   Exception while parsing expression `Variable < 0.40` - 
   Operator '<' incompatible with operand types 'Decimal?' and 'Double'
   

Reproduction

We created a unit test that simulates this race condition:

• Test Name: Promote_Should_Succeed_Even_When_LiteralsCache_Is_Cleaned

• Steps:

  1. Add a constant to the cache.
  2. Simulate cache cleanup by repeatedly calling TryGetText() until _literals is cleaned.
  3. Attempt to Promote() the expression after cleanup.
  4. Expected: Promote() should still return a valid expression (not null).

• Without Fix: The test fails due to missing _literals entries.

• With Fix: Promote() properly handles the absence of _literals.

Proposed Fix

1. Ensure Promote() does not depend entirely on _literals:

   • If TryGetText() fails, fallback to _expressions or recompute the value.

2. Prevent aggressive cleanup of _literals:

   • Increase _literals TTL.
   • Keep _expressions and _literals in sync.

3. Lazy loading mechanism for _literals:

   • If _literals is empty, reconstruct the text from _expressions.

Next Steps

1. Review and confirm the issue using the provided test.
2. Discuss the best approach for fixing this issue.
3. Implement the fix and verify with tests.

    [Fact]
    public async Task Promote_Should_Succeed_Even_When_LiteralsCache_Is_Cleaned()
    {
        // Arrange
        var parsingConfig = new ParsingConfig()
        {
            ConstantExpressionCacheConfig = new Core.Util.Cache.CacheConfig
            {
                CleanupFrequency = TimeSpan.FromMilliseconds(500), // Run cleanup more often
                TimeToLive = TimeSpan.FromMilliseconds(500), // Shorten TTL to force expiration
                ReturnExpiredItems = false
            }
        };
        var constantExpressionHelper = ConstantExpressionHelperFactory.GetInstance(parsingConfig);
        var expressionPromoter = new ExpressionPromoter(parsingConfig);

        double value = 0.40;
        string text = "0.40";
        Type targetType = typeof(decimal);

        // Step 1: Add constant to cache
        var literalExpression = constantExpressionHelper.CreateLiteral(value, text);
        Assert.NotNull(literalExpression); // Ensure it was added

        // Step 2: Manually trigger cleanup
        var cts = new CancellationTokenSource(500);
        await Task.Run(async () =>
        {
            while (!cts.IsCancellationRequested)
            {
                constantExpressionHelper.TryGetText(literalExpression, out _);
                await Task.Delay(50); // Give some time for cleanup to be triggered
            }
        });

        // Ensure some cleanup cycles have passed
        await Task.Delay(500); // Allow cache cleanup to happen

        // Step 3: Attempt to promote the expression after cleanup
        var promotedExpression = expressionPromoter.Promote(literalExpression, targetType, exact: false, convertExpression: true);

        // Assert: Promotion should still work even if the cache was cleaned
        promotedExpression.Should().NotBeNull(); // Ensure `Promote()` still returns a valid expression
    }

Would love feedback on the best approach! 🚀