Jasper: Thread-safety in generated _jspService code

["Vanlerberghe, Luc" <lu...@bvdep.com>]

Hi,

I've taken a look at the servlet code that jasper generates when compiling
my jsp pages.

I noticed the following code fragment near the start of each _jspService
method:
    if (_jspx_inited == false) {
        _jspx_init();
        _jspx_inited = true;
    }

This code is not thread-safe: unless the page is marked as being
SingleThreadModel, multiple threads can attempt to execute the _jspx_init
body. No problem for my pages since _jspx_init happens to be empty, but I
guess I'm just lucky.

I see two solutions: 
1. synchronize access to the flag (with a possible acceleration)
    if (_jspx_inited == false) {
        synchronized(this) { // Any object will do, it just has to be the
same for all threads, forcing them to re-read _jspx_inited from main memory
            if (_jspx_inited == false) {
                _jspx_init();
                _jspx_inited = true;
            }
        }
    }
Note: this double-test trick is only thread-safe if the flag never ever
returns to the false state, otherwise you have to synchronize on every
access to the flag.

2. Move the call to _jspx_init() to the init method of the generated
servlet.  That way it becomes the container's responsibility to make sure
that only one thread runs the initialisation code.



Further browsing through the jasper sources showed that this part of the
code is (for now) only used for cases where a StoredCharDataGenerator is
used.  In that case jasper generates a _jspx_init method that loads a
String[][] object from a file.  It won't hurt Tomcat if this code is called
multiple times, but it's not proper behaviour.


Another suggestion:
By adding a simple test in
org.apache.jasper.compiler.JspParseEventListener.generateHeader(), this part
of the generated code could be skipped for what I guess are the majority of
jsp pages.  Something like:
	if (hasGenerator(InitMethodPhase.class))
	{
		...
	}
around the relevant parts.
with
    private boolean hasGenerator(Class phase) {
        for(int i = 0; i < generators.size(); i++) {
            Generator gen = (Generator) generators.elementAt(i);
            if (phase.isInstance(gen)) {
                return true;
            }
        }
        return false;
    }

Luc Vanlerberghe

P.s.: One day I'll have everything set up properly so that I can suggest
changes using the proper diff files, I promise :) ...

P.p.s.: My config:
Tomcat 3.2b7
Apache 1.3.12
Sun jdk 1.3 standard edition
NT 4.0 SP6a
PIII 500MHz, 256MB

[BUG TRACKING] BugRat core

[Nick Bauman <ni...@cortexity.com>]

BugRat blew chunks last night. I had a core dump and a down znutar this
morning with 32b7. This has never happened before.