Return-Path: <luc.vanlerberghe@bvdep.com>
Mailing-List: contact tomcat-dev-help@jakarta.apache.org; run by ezmlm
Delivered-To: mailing list tomcat-dev@jakarta.apache.org
Delivered-To: moderator for tomcat-dev@jakarta.apache.org
Received: (qmail 96530 invoked from network); 22 Nov 2000 13:25:12 -0000
Received: from smtp.bvdep.com (HELO smtp.bvd) (193.194.158.46)
  by locus.apache.org with SMTP; 22 Nov 2000 13:25:12 -0000
Received: from SMTP ([172.28.2.46]) by smtp.bvd  with Microsoft
 SMTPSVC(5.5.1877.197.19);
	 Wed, 22 Nov 2000 14:25:08 +0100
Received: from mail.bvdep.com ([172.28.2.8]) by 172.28.2.46
  (Norton AntiVirus for Internet Email Gateways 1.0) ;
  Wed, 22 Nov 2000 13:25:08 0000 (GMT)
Received: by MAIL with Internet Mail Service (5.5.2650.21)
	id <WP5VC8Y7>; Wed, 22 Nov 2000 14:25:07 +0100
Message-ID: <2F7BD08CBEC9D311ADA9009027B0FC9C56DC13@MAIL>
From: "Vanlerberghe, Luc" <luc.vanlerberghe@bvdep.com>
To: tomcat-dev@jakarta.apache.org
Subject: Jasper: Thread-safety  in generated _jspService code
Date: Wed, 22 Nov 2000 14:25:06 +0100
MIME-Version: 1.0
X-Mailer: Internet Mail Service (5.5.2650.21)
Content-Type: text/plain;
	charset="iso-8859-1"
X-Spam-Rating: locus.apache.org 1.6.2 0/1000/N

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