Re: Thread-safety
"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
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
> 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
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
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 ""?
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
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