samskivert / jmustache

A Java implementation of the Mustache templating language.
Other
828 stars 129 forks source link

IncludedTemplateSegment thread safety #138

Closed drykovanov closed 7 months ago

drykovanov commented 1 year ago

I'm worried about IncludedTemplateSegment thread safety. https://github.com/samskivert/jmustache/blob/master/src/main/java/com/samskivert/mustache/Mustache.java#L844

Under concurrent load partial template can be loaded multiple time. Also _template seem to be prone to visibility issues.

Maybe I'm missing something though.

drykovanov commented 1 year ago
import java.io.Reader;
import java.io.StringReader;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.concurrent.CyclicBarrier;
import java.util.concurrent.atomic.AtomicInteger;

import static org.junit.jupiter.api.Assertions.fail;
import static org.assertj.core.api.Assertions.assertThat;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

import com.samskivert.mustache.Mustache;
import com.samskivert.mustache.Template;

public class JMustacheThreadSafety {

    private static final String templateA = """
   Template A content
   {{> templateB}}
            """;

    private static final String templateB = """
   Template B content
            """;

    private static class CountingLoader implements Mustache.TemplateLoader {

        private final AtomicInteger counter = new AtomicInteger(0);

        @Override
        public Reader getTemplate(String name) throws Exception {
            counter.incrementAndGet();

            if (!"templateB".equals(name)) {
                throw new IllegalArgumentException();
            }

            return new StringReader(templateB);
        }

        public int counter() {
            return counter.get();
        }
    }

    @ParameterizedTest
    @ValueSource(ints = {1, 2, 4, 8, 16, 32, 256, 1024, 4096})
    void templateLoadingIsThreadSafe(int nThreads) throws InterruptedException {
        final var barrier = new CyclicBarrier(nThreads);
        final var loader = new CountingLoader();
        final Mustache.Compiler compiler = Mustache.compiler().withLoader(loader);
        final Template template = compiler.compile(new StringReader(templateA));

        final List<Thread> threads = new LinkedList<>();
        for (int i = 0; i < nThreads; i++) {
            final Thread thread = new Thread(
                    () -> {
                        try {
                            barrier.await();

                            final String value = template.execute(Map.of());

                            if (!value.contains("Template A content\nTemplate B content")) {
                                fail("Invalid template result " + value);
                            }
                        }catch (Exception e) {
                            throw new RuntimeException(e);
                        }
                    }
            );

            threads.add(thread);
            thread.start();
        }

        for (Thread thread : threads) {
            thread.join();
        }

        assertThat(loader.counter()).isEqualTo(1);
    }
}
org.opentest4j.AssertionFailedError: 
expected: 1
 but was: 15
Expected :1
Actual   :15

org.opentest4j.AssertionFailedError: 
expected: 1
 but was: 6
Expected :1
Actual   :6