Re: Thread-safety

2001-01-30 Thread Luc Vanlerberghe

"Klemme, Robert, myview" wrote:
> what irritates me here is the point that they should not have taken into
> consideration what happens to multithreaded programs.  this is because as
> far as i understand support for multithreaded applications is one of the
> core features of the java language.
Multithreading is a lot easier in Java than in any other language I know
of and so is the synchronization between threads using 'synchronize'
blocks.  The danger lies in people trying to devise clever schemes to
circumvent the costly locking of objects and not realising their code is
not thread-safe because of that.  If you take the conservative approach
and synchronize whenever data is exchanged between threads you're safe. 
Anything else must be studied *very* carefully by someone with
sufficient experience in the field.

> ironically the optimization which was implemented for performance
> improvement does lead to a performance loss (because the locking of objects
> is costly) in another place.  of course, overall this could still be an
> improvement - especially if most of the code executes "single threaded"...
If this optimization was not allowed, then the JVM would effectively be
forced to synchronize on *every single access* to main memory.
If you take into account that usually most of the useful stuff that a
thread does (i.e.: handling some kind of request/method call/whatever)
does not need synchronization since it stays in its own memory space (I
mean, it uses objects or values that no other thread is modifying), the
performance benefits can be enormous.
Only when communicating other thread, you have to use the synchronized
keyword to insert barriers for these optimizations...



Luc Vanlerberghe

"Klemme, Robert, myview" wrote:
> 
> hi!
> 
> > -Original Message-
> > From: Luc Vanlerberghe [mailto:[EMAIL PROTECTED]]
> > Sent: Tuesday, January 30, 2001 10:26 AM
> > To: [EMAIL PROTECTED]
> > Subject: Re: Thread-safety
> >
> >
> > There are IMHO two reasons why these statements may be
> > 'executed' out of
> > order:
> > [...]
> 
> tis is all very reasonable.
> 
> > To summarize:
> > The designers of the JVM spec relaxed the requirements on order of
> > execution to allow as much performance as possible while still
> > guaranteeing the outcome of a program *if it is run in a
> > single thread*
> 
> what irritates me here is the point that they should not have taken into
> consideration what happens to multithreaded programs.  this is because as
> far as i understand support for multithreaded applications is one of the
> core features of the java language.
> 
> > (I believe the word is 'deterministic').
> > Whenever there's communication between threads, the programmer must be
> > *very* careful to ensure that the state seen by one thread corresponds
> > to the one written by another.
> > AFAIK the only mechanism the Java language provides to ensure this is
> > the use of synchronized blocks.
> 
> ironically the optimization which was implemented for performance
> improvement does lead to a performance loss (because the locking of objects
> is costly) in another place.  of course, overall this could still be an
> improvement - especially if most of the code executes "single threaded"...
> 
> regards
> 
> robert
> 
> -
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, email: [EMAIL PROTECTED]


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, email: [EMAIL PROTECTED]




Re: Thread-safety

2001-01-30 Thread Luc Vanlerberghe

There are IMHO two reasons why these statements may be 'executed' out of
order:
1. If the compiler (or the JVM executing the bytecode) can prove that
the reordering won't have any effect *on the current thread* (in this
case, the value is not referenced elsewhere *in the current thread*,
then reordering is allowed (with some restrictions like those concerning
the synchronized blocks). This makes sure that all assignments made by a
thread will be seen in that order by *that* thread.  If there are no
restrictions within a certain amount of code, then the compiler/JVM is
free to reorder those instructions to improve performance.

2. Even if the instructions are executed in order, the memory subsystem
can flush the writes to main memory in another order than the process
wrote them to the memory subsystem (again with certain restrictions).  I
think this case is even easier to visualise than the preceding one.  If,
in a multiprocessor system every write to a variable has to be
accompanied by extensive communication between caches on all levels to
make sure everything happens in order, there would be a serious
performance hit.  I'm not familiar with the architecture of current
multiprocessor systems, but I'm pretty sure that the restrictions on the
order of memory stores has been relaxed as well for the general case,
but that they all have some way to garantee consistency when the need
arises.  Since the Java bytecode runs on an abstract machine, the
designers of the language needed a system to have an abstract way of
specifying this 'consistency' and they included it in the synchronized
statement.

To summarize: 
The designers of the JVM spec relaxed the requirements on order of
execution to allow as much performance as possible while still
guaranteeing the outcome of a program *if it is run in a single thread*
(I believe the word is 'deterministic').
Whenever there's communication between threads, the programmer must be
*very* careful to ensure that the state seen by one thread corresponds
to the one written by another. 
AFAIK the only mechanism the Java language provides to ensure this is
the use of synchronized blocks.

Luc Vanlerberghe

"Klemme, Robert, myview" wrote:
> 
> thank you paul to point me at an omission.
> 
> > -Original Message-
> > From: Paul Speed [mailto:[EMAIL PROTECTED]]
> > Sent: Monday, January 29, 2001 11:54 AM
> > To: [EMAIL PROTECTED]
> > Subject: Re: Thread-safety
> > [...]
> >   The problem is that the point of the code block is to be
> > sure that the _jspx_init() method has been completed before
> > proceeding.  The problem is that _jspx_inited might appear to other
> > threads to be set to true before the original thread has completed
> > executing the _jspx_init() method (or at least before its changes
> > are available to other threads).
> >
> >   This means that the second thread would come through, see
> > that _jspx_inited is true, and skip the synchronization block.  That
> > threads execution would then proceed thinking that everything has
> > been initialized when it hasn't.
> 
> ok, i see.  thank you again for clarifying.  do i now fully understand the
> issue: the problem at hand cannot easily be solved by assigning the flag
> from the return value of the init() method because of a combination of
> inlining and reordering which might again lead to a prior assignment.  is
> that so?
> 
> normally i would say that a code like "_jspx_inited = do_init();" may not be
> rearranged in a way that the assignment occurs prior to finishing of the
> method body (especially since there can be exceptions).  i would guess that
> - by allowing this - a whole bunch of programs would run berserk or simply
> break...  i cannot believe that people at sun would risk these consequences,
> do they?
> 
> >   Check out the article that is referenced in other mail in
> > this thread or hit JavaWorld and see the references section on the
> > article about singletons.
> 
> this one?
> http://www.javaworld.com/javaworld/jw-04-1999/jw-04-toolbox.html
> 
> regards
> 
> robert
> 
> -
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, email: [EMAIL PROTECTED]


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, email: [EMAIL PROTECTED]




Re: Thread-safety

2001-01-26 Thread Luc Vanlerberghe

> Does this mean that the following code would be thread safe?
NO, it's not!

Check the JavaOne session I mentioned and follow the links they gave. 
There have been various patches suggested, but most of them are wrong...
The only safe way to do it is synchronize before the first test...

The reason your construct might fail is that the JLS does not allow
statements to be moved out of a synchronized block, but it is allowed to
move statements into such a block (with certain restrictions)...
So the _jspx_inited = true can be legally moved into the second
synchronized block and then moved around within it again according to
what the compiler/memory-subsysytem thinks best to optimize for speed.

It may work on 99% of all systems/compiler combinations, but Murphy's
law says that chances are still 50/50 it will fail once in a while on
your system while the boss is watching :) 

These optimizations were explicitly allowed in the JL/JVM spec to allow
for speed optimizations in the compilers/JVMs/multiprocessing hardware.

Again, for more info, follow the links from the session for more info...
http://w6.metronet.com/~wjm/tomcat/2000/Nov/msg00747.html)
http://java.sun.com/javaone/javaone00/replay.html)
http://jsp.java.sun.com/javaone/javaone2000/event.jsp?eventId=754

Luc Vanlerberghe



Ethan Wallwork wrote:
> 
> "When the thread exits the synchonized block, it is required to commit all its
> changes to main memory"
> 
> Does this mean that the following code would be thread safe?
> 
> if (_jspx_inited == false) {
> synchronized (this) {
> if (_jspx_inited == false) {
> synchronized(new Object()) {_jspx_init();}
> _jspx_inited = true;
> }
> }
> }
> 
> The inner synchronized block should ensure that the initialization gets
> commited before _jpsx_inited gets set to true.
> 
> Fun stuff!
> 
> --
> Ethan
> 
> -Original Message-
> From: Pier Fumagalli [mailto:[EMAIL PROTECTED]]
> Sent: Friday, January 26, 2001 5:03 AM
> To: [EMAIL PROTECTED]
> Cc: Justyna Horwat
> Subject: Re: Thread-safety
> 
> Luc Vanlerberghe <[EMAIL PROTECTED]> wrote:
> 
> > Thanks for incorporating this change to jasper.  I had suggested it a
> > couple of months ago (22/11/2000 in fact: see
> > http://w6.metronet.com/~wjm/tomcat/2000/Nov/msg00747.html)
> >
> > In the meantime, however, I have been browsing through the sessions of
> > the JavaOne conference of 2000 and there's (at least) one session of
> > particular interest: "Correct and Efficient Synchronization of Threads
> > in the Java Programming Environment" (The complete list including links
> > to realAudio recordings is on
> > http://java.sun.com/javaone/javaone00/replay.html)
> > Here's a direct link to the abstract:
> > http://jsp.java.sun.com/javaone/javaone2000/event.jsp?eventId=754
> >
> > One of the points that is made in that session is that even this
> > 'double-check idiom' is NOT thread-safe!
> > The exact reasons for this are not so easy to understand but basically
> > what I understood is that within the synchronized block, writes to main
> > memory can occur in any order, meaning that the value of _jspx_inited
> > can be commited to main memory while some of the results of the
> > initialisation code are still in e.g. processor registers.  When the
> > thread exits the synchonized block, it is required to commit all its
> > changes to main memory, but if another processor checks the value of
> > _jspx_inited *before* acquiring the lock, it may see _jspx_inited being
> > true while not all other initialisation data has actually been written
> > to main memory.
> > For more details, please follow the links above...
> 
> GOTCHA! I was looking at your post with Justy this afternoon and didn't
> understand why it's not "threadsafe"... What she committed (without the ugly
> writer.println() stuff is:
> 
> if (_jspx_inited == false) {
> synchronized (this) {
> if (_jspx_inited == false) {
> _jspx_init();
> _jspx_inited = true;
> }
> }
> }
> 
> So, if the commit of the _jspx_inited value can occour while _jspx_init() is
> still in progress (let's imagine some weird code optimization techniques),
> to fix this bug, we should simply remove the first line of the commit and
> simply write:
> 
> synchronized (this) {
> if (_jspx_inited == false) {
> _jspx_init();
> _jspx_inited = true;
> }
> }
> 
> So that, no matter what happens, a thread must ALWAYS acquire a lock on the
> synchronized piece of code, and so we are guaranteed that _jspx_

Thread-safety

2001-01-25 Thread Luc Vanlerberghe

Thanks for incorporating this change to jasper.  I had suggested it a
couple of months ago (22/11/2000 in fact: see
http://w6.metronet.com/~wjm/tomcat/2000/Nov/msg00747.html)

In the meantime, however, I have been browsing through the sessions of
the JavaOne conference of 2000 and there's (at least) one session of
particular interest: "Correct and Efficient Synchronization of Threads
in the Java Programming Environment" (The complete list including links
to realAudio recordings is on
http://java.sun.com/javaone/javaone00/replay.html)
Here's a direct link to the abstract:
http://jsp.java.sun.com/javaone/javaone2000/event.jsp?eventId=754

One of the points that is made in that session is that even this
'double-check idiom' is NOT thread-safe!
The exact reasons for this are not so easy to understand but basically
what I understood is that within the synchronized block, writes to main
memory can occur in any order, meaning that the value of _jspx_inited
can be commited to main memory while some of the results of the
initialisation code are still in e.g. processor registers.  When the
thread exits the synchonized block, it is required to commit all its
changes to main memory, but if another processor checks the value of
_jspx_inited *before* acquiring the lock, it may see _jspx_inited being
true while not all other initialisation data has actually been written
to main memory.
For more details, please follow the links above...



One if the questions I raised in my earlier post is why this
initialisation code isn't moved to the init() method of the generated
servlet? This moves the responsability for executing the initialization
exactly once to the container where IMHO it belongs...

Luc Vanlerberghe



[EMAIL PROTECTED] wrote:
> 
> horwat  01/01/24 12:26:39
> 
>   Modified:jasper/src/share/org/apache/jasper/compiler
> JspParseEventListener.java
>   Log:
>   Fix _jspx_init() thread safety
> 
>   BR 157
> 
>   Revision  ChangesPath
>   1.21  +11 -3 
>jakarta-tomcat-4.0/jasper/src/share/org/apache/jasper/compiler/JspParseEventListener.java
> 
>   Index: JspParseEventListener.java
>   ===
>   RCS file: 
>/home/cvs/jakarta-tomcat-4.0/jasper/src/share/org/apache/jasper/compiler/JspParseEventListener.java,v
>   retrieving revision 1.20
>   retrieving revision 1.21
>   diff -u -r1.20 -r1.21
>   --- JspParseEventListener.java2000/12/22 18:37:39 1.20
>   +++ JspParseEventListener.java2001/01/24 20:26:39 1.21
>   @@ -1,7 +1,7 @@
>/*
>   - * $Header: 
>/home/cvs/jakarta-tomcat-4.0/jasper/src/share/org/apache/jasper/compiler/JspParseEventListener.java,v
> 1.20 2000/12/22 18:37:39 pierred Exp $
>   - * $Revision: 1.20 $
>   - * $Date: 2000/12/22 18:37:39 $
>   + * $Header: 
>/home/cvs/jakarta-tomcat-4.0/jasper/src/share/org/apache/jasper/compiler/JspParseEventListener.java,v
> 1.21 2001/01/24 20:26:39 horwat Exp $
>   + * $Revision: 1.21 $
>   + * $Date: 2001/01/24 20:26:39 $
> *
> * 
> *
>   @@ -333,8 +333,16 @@
> writer.println();
>writer.println("if (_jspx_inited == false) {");
>writer.pushIndent();
>   + writer.println("synchronized (this) {");
>   +writer.pushIndent();
>   +writer.println("if (_jspx_inited == false) {");
>   +writer.pushIndent();
>writer.println("_jspx_init();");
>writer.println("_jspx_inited = true;");
>   +writer.popIndent();
>   +writer.println("}");
>   +writer.popIndent();
>   +writer.println("}");
>writer.popIndent();
>writer.println("}");
> 
> 
> 
> 
> 
> -
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, email: [EMAIL PROTECTED]


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, email: [EMAIL PROTECTED]




Re: [PATCH] Tomcat4.0: org.apache.catalina.connector.http.SocketInputStream

2001-01-08 Thread Luc Vanlerberghe

Oops, apparently I tend to misread do {} while's as the pascal
repeat...until statement (not the first time...).  I thought the loop
was skipping until the next CR/LF.

Please ignore my earlier mail...

Luc

"Vanlerberghe, Luc" wrote:
> 
> I noticed the possibility for an infinite loop in
> org.apache.catalina.connector.http.SocketInputStream
> after the commit on thursday...
> 
> I've attached a patch, but the diff is pretty unreadable...
> 
> I changed the following piece of code:
> 
> do { // Skipping CR or LF
> try {
> chr = read();
> } catch (IOException e) {
> chr = -1;
> }
> } while ((chr == CR) || (chr == LF));
> if (chr == -1)
> throw new EOFException
> (sm.getString("requestStream.readline.error"));
> if ((chr != CR) || (chr != LF)) {
> pos--;
> }
> 
> 1. When there's a IOException, there's no way out of the loop...
> 2. After the loop chr is neither CR nor LF, so the test looks obsolete. pos
> should always be decremented.  (I didn't check why.  I only looked at this
> piece of code, not the reasons behind the logic)
> 3. I moved the try {} catch outside the loop to make it a bit clearer (a
> matter of taste, I guess but also I'm not so sure entering/leaving a try {}
> catch doesn't involve a slight overhead)
> 
> This is the result:
> 
> try {
> do { // Skipping CR or LF
> chr = read();
> } while ((chr == CR) || (chr == LF));
> 
> pos--;
> } catch (IOException e) {
> throw new EOFException
>         (sm.getString("requestStream.readline.error"));
> }
> 
> I compiled the code and tested some of the example pages, so I don't think I
> did any damage...
> 
> Luc Vanlerberghe
>  <>
> 
>   
> 
>SocketInputStream.java.diffName: SocketInputStream.java.diff
>   Type: unspecified type (application/octet-stream)
> 
>   
> -
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, email: [EMAIL PROTECTED]


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, email: [EMAIL PROTECTED]




Catalina: HttpProcessor and HttpConnector synchronize on ""?

2000-12-22 Thread Luc Vanlerberghe

Hi,

I noticed something peculiar while browsing through the sources of
Tomcat/Catalina.

Both HttpProcessor and HttpConnector use a private String object to
synchronize their threads on:
private String threadSync = "";

The JLS requires that all static String be 'intern'ed so they all refer
to the same String Object.  See the Java Language Spec. 3.10.5
(http://java.sun.com/docs/books/jls/second_edition/html/lexical.doc.html)

This means that all HttpProcessors and HttpConnectors use the same
object to synchronize on!

Unless this is intentional (which I doubt) it suggest both be replaced
by
private Object threadSync = new Object();

Luc Vanlerberghe

Index: HttpConnector.java
===
RCS file: 
/home/cvspublic/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/connector/http/HttpConnector.java,v
retrieving revision 1.5
diff -u -r1.5 HttpConnector.java
--- HttpConnector.java  2000/12/16 19:01:23 1.5
+++ HttpConnector.java  2000/12/22 15:42:37
@@ -253,7 +253,7 @@
 /**
  * The thread synchronization object.
  */
-private String threadSync = "";
+private Object threadSync = new Object();
 
 
 /**
Index: HttpProcessor.java
===
RCS file: 
/home/cvspublic/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/connector/http/HttpProcessor.java,v
retrieving revision 1.18
diff -u -r1.18 HttpProcessor.java
--- HttpProcessor.java  2000/12/17 01:05:40 1.18
+++ HttpProcessor.java  2000/12/22 15:42:38
@@ -242,7 +242,7 @@
 /**
  * The thread synchronization object.
  */
-private String threadSync = "";
+private Object threadSync = new Object();
 
 
 /**



Re: cvs commit:jakarta-tomcat/src/examples/WEB-INF/classes/examples ShowSource.java

2000-12-11 Thread Luc Vanlerberghe

Wouldn't it be a better idea NOT to expand the contents of the WEB-INF
and META-INF directories along with the rest of the webapp and expand
them into some other directory instead?

Instead of making everything available and try to restrict access
afterwards, it would be much safer not to make it available in the first
place...

In fact the same goes for .jsp pages themselves too.  The container
recognises incoming request for those pages anyway and will know where
to find the source.

The only things that should be left in the webapps directory is any
static content (or the other way round of course, make a separate
directory with all static content and let that be served by e.g. Apache)

I checked the servlet specification (both 2.2 and 2.3pfd) and I don't
see anything that conflicts with this.  If a servlet or .jsp page wants
the static contents of an otherwise dynamic page (such as the source of
a jsp page) it has to use the getResource or getResourceAsStream method
of the ServletContext interface anyway, so the container can return the
correct URL or stream.

I realise this will require some major redesigning, but it would make a
lot of security leaks next to impossible (e.g. misconfiguring apache
wouldn't allow the clients to see any sources even if they inadvertently
have total access to the webapp directory, the problems with
case-sensitivity wouldn't be that security-sensitive any more, etc...).

In a perfect world, Tomcat would scan the web.xml file, determine what
files are actually dynamic content and move these to a separate
directory on the same level as webapps/
However, in a first phase, only the WEB-INF and the META-INF directory
could be moved.


Just firing some ideas...

Luc Vanlerberghe


"Craig R. McClanahan" wrote:
> 
> Jon Stevens wrote:
> 
> > on 12/9/2000 7:07 PM, "[EMAIL PROTECTED]"
> > <[EMAIL PROTECTED]> wrote:
> >
> > > +(jspFile.toUpperCase().indexOf("/WEB-INF/") != 0) ||
> > > +(jspFile.toUpperCase().indexOf("/META-INF/") != 0))
> >
> > Seems like it would be better to define this as a constant somewhere...
> >
> > public static final String WEB_INF = "/WEB-INF";
> >
> 
> I suppose, although there's only one place within the core servlet container
> that these directories are significant (in the module that handles static
> resources), so a constant would only be used once.
> 
> In the case at hand, this is an *application* level component (the ShowSource
> custom tag used on the "source.jsp" page, inherited back from JSDK 2.1 days)
> that is deliberately ignoring the restrictions of the servlet spec, and you
> would not want to make compiling it dependent on the servlet container core
> classes anyway ...
> 
> >
> > Also, I think you should remove the trailing / because the extra character
> > comparison isn't needed and could cause issues if it isn't there (although
> > it probably wouldn't be...). :-)
> 
> Your suggestion would mean I could not have a directory "WEB-INF-stuff" or
> "META-INF-data" in my webapp treated like any other directory.  That's going
> beyond protecting people and into the realm of infringing their freedom :-).
> 
> >
> > -jon
> >
> > --
> > Honk if you love peace and quiet.
> 
> Craig