[Bug]: Inconsistent state after exception

[Rafaellinos]

Describe the bug

ProcessorContext is not properly cleaned up when an exception occurs during predicate evaluation in PredicateBuilder#test(), leading to a possible ThreadLocal memory leak and incorrect validation behavior in subsequent validations on the same thread.

The bug seems to occurs when a predicate throws an exception: the ProcessorContext.Context#remove() method is never called.

Root Cause

In br.com.fluentvalidator.predicate.PredicateBuilder#test():

@Override
public boolean test(final T value) {
    return predicate.test(value);
}

When predicate.test(value) throws an exception, the call stack unwinds without cleaning up the ProcessorContext that was initialized by RuleProcessorStrategy#process().

The problem flow:

1. RuleProcessorStrategy#process() calls ProcessorContext.get().create()
2. Stream operations iterate over collection items, calling PredicateBuilder#test()
3. If a predicate throws an exception, execution jumps out of the stream
4. ProcessorContext.get().remove() is never reached
5. ThreadLocal retains stale context data

To Reproduce

See this complete reproduction case: https://gist.github.com/Rafaellinos/8a5055f8455a9aad32de50be5de4ed08

Steps to reproduce:

1. Create a validator with a collection property that uses a predicate
2. Make the predicate throw an exception (e.g., NullPointerException, custom exception)
3. Execute the validation
4. Observe that ProcessorContext ThreadLocal is not cleaned up
5. Run another validation on the same thread and observe incorrect behavior

Expected behavior

When an exception occurs during predicate evaluation:

1. The ProcessorContext should be properly cleaned up
2. ThreadLocal should be cleared to prevent memory leaks
3. Subsequent validations on the same thread should work correctly
4. No stale context data should persist

Current Workaround

Adding a try-catch block in PredicateBuilder#test() and returning false:

@Override
public boolean test(final T value) {
    try {
        return predicate.test(value);
    } catch (Exception ex) {
        return false; // Suppresses exception but prevents context leak
    }
}

However, this workaround:

• ❌ Swallows exceptions silently (bad practice)
• ❌ Loses valuable error information
• ❌ Makes debugging difficult
• ✅ Prevents the ProcessorContext leak (but not ideal)

Environment

• Java Version: Java 8, Java 11, Java 17, Java 21 (reproduced across multiple versions)
• Library Version: 1.10.0
• OS: Windows 11, but issue is platform-independent
• Build Tool: Maven 3.x

Proposed Solutions

I'm willing to help fix this issue and open a PR. Here are possible approaches:

Option 1: Try-Catch with Context Cleanup in PredicateBuilder

@Override
public boolean test(final T value) {
    try {
        return predicate.test(value);
    } catch (Exception ex) {
        ProcessorContext.get().remove(); // Clean up context
        throw ex; // Re-throw to preserve exception handling
    }
}

Pros:

• Simple and straightforward
• No external dependencies
• Preserves exception propagation

Cons:

• Mixes infrastructure concerns with business logic
• Need to add similar cleanup in multiple places

Option 2: Try-Finally in RuleProcessorStrategy

Wrap the stream operations in try-finally blocks:

public <E> boolean process(final Collection<E> values, final Rule<E> rule) {
    ProcessorContext.get().create();
    try {
        final boolean allMatch = values.stream().map(value -> {
            ProcessorContext.get().inc();
            return this.process(value, rule);
        }).collect(Collectors.toList()).stream().allMatch(result -> result);
        return allMatch;
    } finally {
        ProcessorContext.get().remove(); // Always executes
    }
}

Pros:

• Guarantees cleanup even on exceptions
• Localized fix in one class
• Standard Java pattern for resource cleanup

Cons:

• Multiple methods need the same pattern

Option 3: AspectJ Advice (Current Attempted Approach)

Use @AfterThrowing aspect to clean up context on exceptions:

@Aspect
public class ValidationExceptionAdvice {
    @AfterThrowing("execution(* *(..)) && @annotation(cleanProcessorValidationContextException)")
    public void afterThrowing(final CleanProcessorValidationContextException annotation) {
        ProcessorContext.get().remove();
    }
}

Pros:

• Separation of concerns
• Cross-cutting concern handled elegantly

Cons:

• Adds AspectJ dependency for library consumers
• More complex setup and classpath requirements
• Causes NoClassDefFoundError: org/aspectj/lang/NoAspectBoundException in consumer projects

Option 4: AutoCloseable Pattern with Try-With-Resources

Refactor ProcessorContext.Context to be AutoCloseable:

public static final class Context implements AutoCloseable {
    // ...existing code...
    
    @Override
    public void close() {
        remove();
    }
}

// Usage:
public <E> boolean process(final Collection<E> values, final Rule<E> rule) {
    try (Context context = ProcessorContext.get().createAndReturn()) {
        return values.stream()
            .map(value -> {
                context.inc();
                return this.process(value, rule);
            })
            .collect(Collectors.toList())
            .stream()
            .allMatch(result -> result);
    }
}

Pros:

• Idiomatic Java resource management
• Guaranteed cleanup
• Clear resource lifecycle

Cons:

• Requires API changes
• More invasive refactoring

Offer to Contribute

I'm happy to:

• 🔧 Implement the fix and open a Pull Request
• ✅ Add comprehensive unit tests reproducing the bug
• 📚 Update documentation with best practices
• 🧪 Add tests for Java 21 compatibility (including preview features if desired)
• 🚀 Help with Java 21 official support if the project wants to support it

I've already tested this library with Java 21 and can contribute tests to officially support it.

Please let me know which solution approach you prefer, and I'll be happy to implement it!

References

• Gist with full reproduction case: https://gist.github.com/Rafaellinos/8a5055f8455a9aad32de50be5de4ed08
• Related AspectJ dependency issue (if Option 3 is pursued): AspectJ runtime must be transitive dependency
• Java ThreadLocal documentation: https://docs.oracle.com/javase/8/docs/api/java/lang/ThreadLocal.html

---

Note: I'm not entirely sure if try-catch/try-finally is the best approach versus using AspectJ. I'm open to discussion and guidance from the maintainers on the preferred architectural direction for this fix.

[zerobak13]

Thanks for the detailed analysis and repro case.

I agree the core issue is that ProcessorContext isn’t cleaned up when a
predicate throws an exception, causing ThreadLocal state to leak.

Option 2 (try–finally cleanup in RuleProcessorStrategy) seems like the
cleanest approach without changing public APIs.
If that sounds good, I can work on a PR with a test to cover this case.