antlr / stringtemplate4

StringTemplate 4
http://www.stringtemplate.org
Other
951 stars 231 forks source link

Bug: ST is NOT threadsafe, even using new ST(ST) doesn't seem to prevent attribute conflicts. #275

Closed rwperrott closed 3 years ago

rwperrott commented 3 years ago

"Javadoc.stg"

//
// Javadoc helper functions
//
docName(name) ::= <<
     * @param <name> .
>>
docStart(name) ::= <<
     * @param <name>Start must be more than or equals to 0.
>>
docEnd(name) ::= <<
     * @param <name>End must be more that or equals to <name>Start.
>>
docLen(name) ::= <<
     * @param <name>Len must be more that or equals to 0, and less than or equals to <name> length - targetStart.
>>
//
doc(of,name,suffixes) ::= <<
     * @param <name> .
<suffixes: {suffix |
<(["doc",suffix])(name)>}>
>>

TypeStrings.stg

import "def.stg"
import "Start.stg"
import "End.stg"
import "Len.stg"
import "Len2.stg"
import "get.stg"
import "Javadoc.stg" // Is listed in  STGroup::.getImportedGroups()

ofFunctions(type,of) ::= <<
    //
    // <Type(type,of)> startsWith <Type(type,of)>
    //
    /**
<doc(of,"target",["Start","Len"])>
     */
    private static boolean startsWith0(<def0(type,of,"source",[])>,
                                       <def0(type,of,"target",["Start","Len"])>,
                                       int fromIndex) {
...

Error Message:

context [/ofsFunctions /_sub2 /ofFunctions] 14:1 passed 3 arg(s) to template null with 1 declared arg(s)

Why can't "TypeStrings.stg" line 14 see "doc" template in "Javadoc.stg"?

The issue disappears when I comment out the Javadoc.stg import line and paste Javadoc.stg content below the import line, hmm!

rwperrott commented 3 years ago

Using:

synchronized (stGroup) {
     st = new ST(stGroup.getInstanceOf(name));
}

I'm still seeing errors like, this, so it still looks hard to find some way to run ST instances concurrently, which is ridiculous.

context [/ofsFunctions /_sub2 /ofFunctions] 14:1 internal error: java.lang.IllegalArgumentException: Formal argument it already exists.
    at org.stringtemplate.v4.compiler.CompiledST.addArg(CompiledST.java:208)
    at org.stringtemplate.v4.ST.add(ST.java:245)
    at org.stringtemplate.v4.Interpreter.storeArgs(Interpreter.java:640)
    at org.stringtemplate.v4.Interpreter._exec(Interpreter.java:233)
    at org.stringtemplate.v4.Interpreter.exec(Interpreter.java:151)
    at org.stringtemplate.v4.Interpreter.writeObject(Interpreter.java:755)
    at org.stringtemplate.v4.Interpreter.writeObjectNoOptions(Interpreter.java:687)
    at org.stringtemplate.v4.Interpreter._exec(Interpreter.java:291)
    at org.stringtemplate.v4.Interpreter.exec(Interpreter.java:151)
    at org.stringtemplate.v4.Interpreter.writeObject(Interpreter.java:755)
    at org.stringtemplate.v4.Interpreter.writeIterator(Interpreter.java:785)
    at org.stringtemplate.v4.Interpreter.writeObject(Interpreter.java:760)
    at org.stringtemplate.v4.Interpreter.writeObjectNoOptions(Interpreter.java:687)
    at org.stringtemplate.v4.Interpreter.toString(Interpreter.java:1163)
    at org.stringtemplate.v4.Interpreter._exec(Interpreter.java:357)
    at org.stringtemplate.v4.Interpreter.exec(Interpreter.java:151)
    at org.stringtemplate.v4.ST.write(ST.java:443)
rwperrott commented 3 years ago

All these problems go away when I only use an ST on one Thread or access ST inside synchronized (stGroup) { }, thus ST has some concurrency issues.

parrt commented 3 years ago

Hi. Yes, I believe we state somewhere that you should not assume that these are thread safe working on the same ST group.

It should be thread safe for one thread and one template instance. Of course if you have two threads trying to manipulate the same object, there's going to be a problem.

rwperrott commented 3 years ago

I suspect that STGroup isn't thread-safe either because the issue seems to occur in referenced templates; so much of STGroups would probably have to be copied to make STs Thread-safe. This is a bit sad, because it ironically makes String Template less scalable than impure templating frameworks like JSP with tag libraries like JSTL.

parrt commented 3 years ago

Well, it has been a while, but I remember making a few very specific choices. For example, check all the comments here for usage etc. https://github.com/antlr/stringtemplate4/issues/61

rwperrott commented 3 years ago

I see, 6 years ago, and an STGroup per Thread was the stated lazy workaround, but still no concurrency, hmm. If it's that hard to do, that strongly suggests a refactor is needed to pay off technical debt.

I think it would be far better to concurrently cache compiled templates in immutable objects, with an attached concurrent cache for lazy resolution of any imports, than to have to keep redundantly repeating the same work; Errors could also be cached to avoid time wasting for subsequent requests. If a synchronized cache(s) is used for a prototype, it could be converted to use a cheaper concurrent barrier, like a StampedLock, or by using ConcurrentHashMap.computeIfAbsent to atomically, lazy create, the requested content; lambdas and streams in Java 1.8+ can make this code much less verbose.

sharwell commented 3 years ago

If we ever wrote a StringTemplate 5, it would basically be the same as StringTemplate 4 but with a new immutable model that is inherently both performant and thread safe. I'm not sure we could truly deliver on the expectations for multi-threaded use without a rewrite though.

Clashsoft commented 3 years ago

@sharwell I agree, making the code thread safe with an immutable model is not possible without breaking changes (e.g. all the public non-final fields in #240).

Btw, will there ever be an ST5? I see a bunch of issues closed with the reasoning that the current code base does not allow it (e.g. this one, multi-char delimiters #90, ...). Could we have a way to track issues that are justified but can't be solved in v4? Maybe a v5 milestone? In case someone ever wants to do a rewrite.

parrt commented 3 years ago

Hi @Clashsoft. Sam and I have no immediate plans for a ST 5.