Template.render() leaving Executor threads active

[mehtasankets]

Template.render() method is creating a separate thread which is not getting closed explicitly. Method should ideally call executorService.shutdown() explicitly to properly close the thread.

Not closing threads explicitly is causing multiple threads to be alive in the system and causes problems at the time of closing the process.

[bkiers]

Thanks @mehtasankets!

[mehtasankets]

Welcom @bkiers .. Can we have new version published with this change?
Also, per my understanding this is a temporary fix and ideally the method should accept ExecutorService so that thread pool and thread management can be pushed back to the client using the library instead of library creating new threads on its own. Any thoughts?

[bkiers]

@mehtasankets ah, yeah, that would be nice. How about something like this: bf4a522 (on a separate branch for now)

[mehtasankets]

@bkiers yes, above fix looks good to me..
Would you be able to publish a new jar with that change?

[bkiers]

The current release, 0.7.2, already has the shutdown of the exe-service:

    public String render(final Map<String, Object> variables) {

        if (this.templateSize > this.protectionSettings.maxTemplateSizeBytes) {
            throw new RuntimeException("template exceeds " + this.protectionSettings.maxTemplateSizeBytes + " bytes");
        }

        final LiquidWalker walker = new LiquidWalker(new CommonTreeNodeStream(root), this.tags, this.filters, this.parseSettings.flavor);

        ExecutorService executorService = Executors.newSingleThreadExecutor();

        Callable<String> task = new Callable<String>() {
            public String call() throws Exception {
                try {
                    LNode node = walker.walk();
                    Object rendered = node.render(new TemplateContext(protectionSettings, renderSettings, parseSettings.flavor, variables));
                    return rendered == null ? "" : String.valueOf(rendered);
                }
                catch (Exception e) {
                    throw new RuntimeException(e);
                }
            }
        };

        Future<String> future = executorService.submit(task);
        try {
            return future.get(this.protectionSettings.maxRenderTimeMillis, TimeUnit.MILLISECONDS);
        }
        catch (TimeoutException e) {
            throw new RuntimeException("exceeded the max amount of time (" +
                    this.protectionSettings.maxRenderTimeMillis + " ms.)");
        }
        catch (Throwable t) {
            throw new RuntimeException("Oops, something unexpected happened: ", t);
        }
        finally {
            executorService.shutdown();
        }
    }

Or do you want a new release of master?

[mehtasankets]

I would actually need a new version with
Future<String> future = executorService.submit(task);
inside try-catch. My application actually broke at that line.

Would you kindly provide a new jar?

[bkiers]

If you only need the JAR, you can assemble it yourself by doing a mvn clean install in the root of the project (it will be located here: ./target/liqp-0.7.3-SNAPSHOT.jar).

If you want a new version in in Maven Central, I'll need to ask someone else.

[mehtasankets]

I would need it via maven central as I have different environments where I run my program and otherwise I'll have to install it everywhere.

[bkiers]

OK, I've asked for a new release to Maven Central: #72

[mosabua]

http://repo1.maven.org/maven2/nl/big-o/liqp/0.7.3/