Is this caching a closure thread save?

Question

public class ExampleServlet extends HttpServlet{

    private Closure closure;

    @Override
    protected void doPost(final HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
        // Do some stuff here...
        if (this.closure == null){
            this.closure = new Closure(){
                @Override
                public void someFunction(){
                    req.getRequestURI(); // Obviously do something with it...
                }
            }
        }

        // Later...
        this.closure.someFunction(); // << Is this thread-save??

        // More stuff here...
    }
}

In the example, I have no control over Closure!

If I test this, it works fine. The question is:

Does every request-thread get a new copy of the closure-field? Otherwise, when a new request comes in, the referenced req-field would change while the someFunction() is still executing. Or is this handled by declaring req as final?

The `if (this.closure == null){ .. }` part is not safe because it needs synchronization :) — zapl, Dec 06 '14 at 00:49

score 3 · Accepted Answer · edited May 23 '17 at 11:58

The same servlet instance is used by all requests.

So the closure will be created for only the first request and shared among all subsequent requests.

It will continue to refer to the initial request that hit the servlet, so this should not work at all, even without multiple threads (single-threaded sequential requests are also broken).

If you do have multiple threads you get extra complications, such as having to synchronize on the servlet instance for the "lazy singleton initialization" part. Ideally, all such initialization is done in the servlet's init method instead.

Cannot you make this a local variable inside of doPost?

Does every request-thread get a new copy of the closure-field?

No. They all share the same servlet, with its (single) closure field.

when a new request comes in, the referenced req-field would change while the someFunction() is still executing.

No. The captured req does not change for the lifetime of the closure. It is final and copied from the servlet when the closure is first created. It will continue to refer to the very first request that hit the servlet.

Or is this handled by declaring req as final?

That is a technical requirement due to the nature of how closures in Java are implemented. They don't really capture variables, they capture snapshot values of variables as a copy.

You're actually right, the code worked because it's in the base class of the actual servlets. So there was actually an instance per Servlet. And the logic in the method doesn't need to be re-evaluated every time, so the results make sense, too. Thanks for clarifying! — Lukas Knuth, Dec 06 '14 at 02:34
@LukasKnuth: That is very brittle. You could map the same Servlet for multiple URL patterns for example. I imagine that would break your code. Try to move this configuration into the init method. — Thilo, Dec 06 '14 at 02:53
I re-create the closure on every call now, it's not that much overhead and i fully understand the code ;) — Lukas Knuth, Dec 06 '14 at 16:51

Is this caching a closure thread save?

1 Answers1