Bug 33453 - Jasper should recompile JSP files whose datestamps change in either direction (not just newer)
Jasper should recompile JSP files whose datestamps change in either direction...
Status: RESOLVED FIXED
Product: Tomcat 5
Classification: Unclassified
Component: Jasper
5.5.15
All All
: P3 enhancement with 11 votes (vote)
: ---
Assigned To: Tomcat Developers Mailing List
:
: 15417 40420 (view as bug list)
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2005-02-08 22:50 UTC by Jamie Flournoy
Modified: 2011-06-20 18:03 UTC (History)
3 users (show)



Attachments
Changes to Compiler.java and JspCompilationContext.java to fix bug 33453 (10.23 KB, application/octet-stream)
2005-09-22 17:35 UTC, Jonathan Leech
Details
Changes to JspCompilationContext.java (1.54 KB, patch)
2005-09-22 21:45 UTC, Jonathan Leech
Details | Diff
Change to Compiler.java (538 bytes, patch)
2005-09-22 21:46 UTC, Jonathan Leech
Details | Diff
rebuilt jasper-compiler.jar (376.44 KB, application/octet-stream)
2005-10-05 17:16 UTC, Jonathan Leech
Details
Compiler.java diff (2.35 KB, patch)
2006-03-23 20:10 UTC, Jonathan Leech
Details | Diff
Generator.java diff (3.33 KB, patch)
2006-03-23 20:11 UTC, Jonathan Leech
Details | Diff
JspSourceDependent.java diff (547 bytes, patch)
2006-03-23 20:11 UTC, Jonathan Leech
Details | Diff
JspServletWrapper.java diff (975 bytes, patch)
2006-03-23 20:12 UTC, Jonathan Leech
Details | Diff
Proposed patch for Tomcat 7 (17.30 KB, patch)
2011-05-20 16:26 UTC, Mark Thomas
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jamie Flournoy 2005-02-08 22:50:23 UTC
I've noticed that Tomcat won't recompile a JSP file if the date stamp is changed
to go back in time. This may seem like a strange case, but if you check an older
copy of a JSP page out of version control, it's different, and needs to be
recompiled. The assumption that all changes to a file involve a newer file
datestamp is an invalid one.

I think I found the code that makes this decision, in
org.apache.jasper.compiler.Compiler, in the isOutDated(boolean) method. The
condition is
        if (targetLastModified < jspRealLastModified)
but it should be
        if (targetLastModified != jspRealLastModified)
in my opinion.

After all, the logic should be that the file has changed, not that it's newer. I
don't think it's reasonable to expect that Jasper check the size and do an MD5
checksum to *really* see if the file has changed. :)

Obviously the workaround is to just to "touch" the file but this adds a lot of
overhead (and one more thing to remember), compared to changing a > to a !=.
Comment 1 Yoav Shapira 2005-02-08 22:55:06 UTC
Changes to a file that make the change timestamp older instead of newer seems 
like a special (if not outright wrong) use-case.  I'm not sure Tomcat should 
worry about this at all.
Comment 2 Jamie Flournoy 2005-02-08 23:03:23 UTC
(In reply to comment #1)
> Changes to a file that make the change timestamp older instead of newer seems 
> like a special (if not outright wrong) use-case.

I don't agree that using version control is a special or wrong use case. In
fact, I think the opposite is true.

Example:
1) Check out a JSP file from VC and deploy it where Tomcat can see it
2) Access the JSP file via a web browser, so it gets compiled
3) Change it locally, deploy the changed version (newer timestamp)
4) Access it via a web browser; Tomcat correctly shows the updated version.
5) Revert to an older version from VC and deploy that.
6) Access it via a web browser; Tomcat ignores the changes and shows the output
of a nonexistent JSP page


Comment 3 Tim Funk 2005-02-09 00:04:43 UTC
This is an enhancement at best. (That I would be -0 to)
Comment 4 Rainer Jung 2005-02-09 00:46:40 UTC
Although not allowed to vote I have an opinion: The dynamic update feature for
JSPs comes from the need to dynamically manage content without a heavy weight
administrative action.

But every now and then one has to roll back a change. With static content you
would just put back the original files. It would be very nice, if it would work
the same way with JSPs, especially because that's unsually the unplanned case
(roll back) where you need to have a simple procedure.

I know, you could touch the old file. Myself I always tell Sysadmins to make
backup copies and roll back file changes with "cp -p", so that the files keep
their original timestamp, which is a nice low-level approximation of checksums.
Any higher level tool (like CVS) will most liekely also roll back including file
timestamps.

There seems to be very little risk in the change and some not neglectable benefit.
Comment 5 Remy Maucherat 2005-02-09 01:02:39 UTC
Please test this *really* well. Also, you should test with a similar change made
to the dependent files near the end of the isOutDated method (otherwise, touch
would still be required in some cases).
Comment 6 Rainer Jung 2005-02-09 02:03:12 UTC
Thanks for your hint.

I do like the use case, but I should have thought better about the
implementation suggestion of Jamie. His idea to just compare for timestamps for
not being equal will not work!

In most cases the timestamps of the class files will of course be younger than
the source file (source file has some old installation date but class file is
only generated on first access). So this change would make the compiler compile
on every test :(

It would only work by saving the last JSP timestamp for any JSP and then
comparing to the saved value insted of comparing to the timestamp of the
generated files. I leave it up to Jamie to suggest a working patch - I don't knw
enough about Jasper details.
Comment 7 Jamie Flournoy 2005-02-09 04:03:03 UTC
Fair enough; that's not the place to make that change. The servlet .class or
.java file will be slightly newer than the JSP file. It's the JSP file's date
that should be compared to the cached last-modified time, regardless of how many
ms it took to get around to generating and compiling the actual servlet.

Comment 8 Yoav Shapira 2005-02-18 15:08:45 UTC
Feel free to reopen this when you have a patch ready for us to evaluate -- 
looking forward to it.
Comment 9 Jonathan Leech 2005-09-22 01:15:02 UTC
The .jsp file date stamp doesn't have to go back in time for the isOutDated
check to fail, it can and does fail in a more normal usage pattern.
Here is a scenario that shows the problem:
- I deploy version 1, the .jsp has time1
- I make version 2 of the .jsp at time2
- Visitor visits the site, and the .jsp is compiled at time3
- I deploy version2
- isOutDated returns false as time3 > time2

Would setting the date stamp of the .java and .class files to the date stamp of
the .jsp file, and changing the comparison from < to != in the isOutDated check
fix the problem sufficiently?  Or are there negative side effects I haven't
thought of?

I am working on patching my Tomcat to do exactly as above, I would be happy to
give it to someone for evaluation when its ready.  
Comment 10 Remy Maucherat 2005-09-22 07:59:47 UTC
(In reply to comment #9)
> The .jsp file date stamp doesn't have to go back in time for the isOutDated
> check to fail, it can and does fail in a more normal usage pattern.
> Here is a scenario that shows the problem:
> - I deploy version 1, the .jsp has time1
> - I make version 2 of the .jsp at time2
> - Visitor visits the site, and the .jsp is compiled at time3
> - I deploy version2
> - isOutDated returns false as time3 > time2
> 
> Would setting the date stamp of the .java and .class files to the date stamp of
> the .jsp file, and changing the comparison from < to != in the isOutDated check
> fix the problem sufficiently?  Or are there negative side effects I haven't
> thought of?
> 
> I am working on patching my Tomcat to do exactly as above, I would be happy to
> give it to someone for evaluation when its ready.  

As looked into in comment #6, this is not doable easily, which makes the few use
cases which could benefit from this not worth it. Please try to read the report
next time.
Comment 11 Jonathan Leech 2005-09-22 17:32:57 UTC
Remy,
With all due respect I did read the report fully and I believe the
recommendation I made addressed comment #6.  If you feel my recommendation isn't
sufficient, please state why.
The other point in my first comment was that this bug can manifest itself in
ways more common than the original report, in fact that's why my colleague and I
found it.  
I have patched and tested my local Tomcat, and am attaching the two files I
modified for review.

(In reply to comment #10)
> (In reply to comment #9)
> > The .jsp file date stamp doesn't have to go back in time for the isOutDated
> > check to fail, it can and does fail in a more normal usage pattern.
> > Here is a scenario that shows the problem:
> > - I deploy version 1, the .jsp has time1
> > - I make version 2 of the .jsp at time2
> > - Visitor visits the site, and the .jsp is compiled at time3
> > - I deploy version2
> > - isOutDated returns false as time3 > time2
> > 
> > Would setting the date stamp of the .java and .class files to the date stamp of
> > the .jsp file, and changing the comparison from < to != in the isOutDated check
> > fix the problem sufficiently?  Or are there negative side effects I haven't
> > thought of?
> > 
> > I am working on patching my Tomcat to do exactly as above, I would be happy to
> > give it to someone for evaluation when its ready.  
> 
> As looked into in comment #6, this is not doable easily, which makes the few use
> cases which could benefit from this not worth it. Please try to read the report
> next time.
> 

Comment 12 Jonathan Leech 2005-09-22 17:35:51 UTC
Created attachment 16489 [details]
Changes to Compiler.java and JspCompilationContext.java to fix bug 33453

Modified Compiler.java isOutDated() method to use != instead of > for datestamp
comparison.
Modified JspCompilationContext.java compile() method to set the datestamp of
the generated .java and .class files to the datestamp of the source .jsp.
Comment 13 Jonathan Leech 2005-09-22 18:23:24 UTC
Remy,
I will keep re-opening this bug until you take the time to explain to me why I
am wrong.  Which even you can't, because I'm not.
Comment 14 Remy Maucherat 2005-09-22 20:18:07 UTC
Sorry for letting you down, I was having dinner.
Comment 15 Jamie Flournoy 2005-09-22 21:05:40 UTC
Since when is it a good idea to *close* a bug that you can't think of how to fix
offhand, and don't feel like fixing yourself?

This behavior is infantile and is an embarassing contradiction to the spirit of
the open source development model. And now you're refusing to look at a patch
just to save face? This is really sad.

I wonder how many other Tomcat bugs exist but were closed because somebody
didn't want to think about them or try to resolve them.

Maybe you ought to use the priority and target milestone features instead of
pretending that Tomcat does the right thing because whatever it currently does
defines what "the right thing" is.

I'm not going to reopen this because I'm not wasting any more time on this
little power game. There are competitors to Tomcat and this is just another
reason to use them instead.
Comment 16 Jonathan Leech 2005-09-22 21:45:35 UTC
Created attachment 16492 [details]
Changes to JspCompilationContext.java

Sets the lastModified on the generated .java and .class files to the
lastModified of the source .jsp
Comment 17 Jonathan Leech 2005-09-22 21:46:45 UTC
Created attachment 16493 [details]
Change to Compiler.java

Change the < to != in the isOutDated method.
Comment 18 Jonathan Leech 2005-09-22 22:00:30 UTC
As suggested by Rainer, I have submitted my recommended changes in patch form
for easier review.

Remy, I was also at lunch so unfortunately I was delayed in re-opening the bug,
which I likewise apologize for.  I accept your apology. I am considering
creating a script to automate my re-opening of the bug.  As a peace offering I
am willing to automate your part, automatically changing the status to RESOLVED
WONTFIX, just send me your bugzilla login and password.

By the way, if you bothered to take the time to think about the problem, you
would realize as I have that the current behavior is very broken.  As I have
stated before, not just for the original use case in the bug report, it affects
every .jsp modification, without moving the timestamp backward whatsoever.  This
bug affects everyone, period.
 
Comment 19 Mark Thomas 2005-09-22 23:27:17 UTC
My instinct is that changing the true timestamp of generated files is going to
cause other problems. I am very uneasy with a solution that means what a user
sees (in terms of timestamps) isn't going to be what actually happened.

It is also worth having a look at bug 23406. In that case timing resolutions to
the nearest second were not sufficient to resolve the issue. Any patch for this
issue should also address the issue in 23406.

My preference would be for a patch that recorded, for each JSP, somthing that is
guaranteed to change for all of the related use cases. Timestamp + file size
should be OK, MD5 certainly would be.

My general unease about changing file timestamps hasn't got to the point where I
would -1 this patch but I haven't had time to reflect on this yet.

In summary:
- I agree this is a problem
- I agree the right solution isn't going to be easy
- I think we need a more robust solution that the current patch.
Comment 20 Remy Maucherat 2005-09-22 23:33:40 UTC
(In reply to comment #19)
> In summary:
> - I agree this is a problem

I disagree.

> - I agree the right solution isn't going to be easy

I do not wish to find any solution to this non issue. The only problem is that
people will have to use touch or similar in a few very select situations, which
apparently is too difficult.

> - I think we need a more robust solution that the current patch.

Yes, -1 for it. Obviously, anyone is free to waste his time on this trying to
find an acceptable solution, but the said solution has better be trivial,
otherwise it will get a -1 from me.
Comment 21 Rainer Jung 2005-09-22 23:43:57 UTC
Setting timestamp seems not a clean solution to me. API doc says, hat the time
might be rounded, so even if we try to set the time to the same timestamp we get
from the JSP, the resulting timestamp might differ and not be equal to the
original time (Consider diffrent file system types etc.).

I would also prefer a solution where information about the JSP is saved and
later compared. Would JspServletWrapper be the right place to save the original
JSP modification time?

MD5 would be nice, but then md5 checksum would need to be recalculated on every
JSP check with unchanged file time, so unfortunately not a rare case. I guess
that's too bad for performance.

Maybe timestamp and size would be enough, because both can be retrieved easy and
efficiently, and if timestamp did not change, but content did change, it is very
likely, that the file was in progress of being written to, so at least size
should have changed.

If we agree, that it's worth trying to make a patch to JspServletWrapper, I'll
try to submit one tomorrow (not really for 5.5.12).

One thing remains though: I'm not sure how to handle the case of included JSPs
(dependecies). Maybe I'll find a solution by digging deeper into Jasper.

One last word: I had customers having problems with both scenarios: rolling back
file changes, but also distributing content with wrong timestamps (future time)
and in consequence continuous recompilation for several minutes. Not trying to
assume a simple time model seems to make jasper more robust.
Comment 22 Remy Maucherat 2005-09-23 00:13:14 UTC
(In reply to comment #21)
> I would also prefer a solution where information about the JSP is saved and
> later compared. Would JspServletWrapper be the right place to save the original
> JSP modification time?

Nope, people can restart the container.

> MD5 would be nice, but then md5 checksum would need to be recalculated on every
> JSP check with unchanged file time, so unfortunately not a rare case. I guess
> that's too bad for performance.

Arg MD5.

> Maybe timestamp and size would be enough, because both can be retrieved easy and
> efficiently, and if timestamp did not change, but content did change, it is very
> likely, that the file was in progress of being written to, so at least size
> should have changed.

This is simple, and maybe acceptable, but would make the cost of checking for
recompilation (even) more expensive than it is right now.

> One last word: I had customers having problems with both scenarios: rolling back
> file changes, but also distributing content with wrong timestamps (future time)
> and in consequence continuous recompilation for several minutes. Not trying to
> assume a simple time model seems to make jasper more robust.

On access compilation and its friend the development mode - which you are using
or you would not have this "issue" - should not be used in production (the only
reason why it is not as bad as it used to be is that I tweaked it do do only one
check at most per page per time interval - obviously if there are 100 pages, my
trick will not work that well).
Comment 23 Jonathan Leech 2005-09-23 00:32:10 UTC
You disagree because you don't fully understand the problem.  If someone first
visits your .jsp after your modification, but before redeployment, you will be
hit by this bug.  You will wonder why your change didn't take effect.  It is not
necessary for the timestamp to move backwards.  You don't control when your .jsp
is accessed.  As your development/test servers will see different access than
your production, you will encounter a production bug that you didn't see in your
other environments.  Or if you are load balanced you will encounter the bug on
one machine but not another.  Good luck debugging that when it happens to you.

Using touch is not difficult, just add it to the documentation for JBoss and
every other web server that to hot deploy, do the following:  hot deploy, find
the tmp directory where the war is unpacked, touch every .jsp file that changed.
   And do that atomically so that noone can visit the .jsp inbetween.

The jasper code as is fails badly, my fix is trivial, is an improvement, but
still not 100% correct and has the side effect of changing a couple timestamps.
 The only place I'm aware of in Jasper that uses those timestamps is the broken
isOutDated logic.  

My fix doesn't handle dependencies correctly, and there is still the possibility
the .jsp is visited in the same second / minute / whatever the OS granularity is
as the modification.  Another side effect of my fix is that every .jsp will be
recompiled once after the fix is applied.

Timestamp rounding by the filesystem is not an issue, the timestamp of the .jsp
will be rounded the same way as the .class and .java files.  Every OS will round
timestamps in an internally consistent manner.

If changing the timestamps of the .java and .class files is still deemed
verboten, then I suggest copying the .jsp to the same temporary directory that
the .java and .class files are generated in, and preserving its timestamp.  Then
 timestamp, filesize, md5, even the exact contents of the file can be compared.

Remy, I am using JBoss4.0.2, how should I be deploying .jsps to production to
avoid this issue?
Comment 24 Scott Johnson 2005-09-23 01:01:06 UTC
For what it's worth:
A few years ago we implemented the timestamp approach to this issue in the
WebSphere Application Server JSP container at the request of a small number of
customers - for whom it was critical.  A generated classfiles is set to the
timestamp of the source JSP file.  The classfile is considered to be outdated
when the two timestamps do not match.  File size is not part of the equation.
Tag files are handled the same way.
The timestamp disconnect :) of classfiles vs. their actual compilation time
rarely causes confusion among customers; it's a non-issue. We write compilation
time/date and other information into the generated .java file, in a comment, so
any confusion that might occur can be easily cleared up.
The timestamp != strategy has worked well on Versions 4 through 6, on all
platforms. Dependency tracking (static includes, TLDs, tag files) is easily
managed. The race condition described in bug 23406 has never been reported. 
Timestamp rounding has never been an issue.
Google for "websphere jsp timestamp" and you'll find some info about the
implementation.
Some things to consider if you all decide it's an appropriate change for Tomcat
(this stuff is all documented and easily found on the web):
When serving JSP sources from JARs, we use the timestamp of the JAR for the
outdated check.
Any expansion of WARs or other compressed artifacts with precompiled JSP classes
must maintain timestamps (doh).
There *will* be first-time recompilation cost to Tomcat users if this is
implemented, as Jonathan mentioned.  Some won't like it.  
Keeping data only in a runtime artifact like the servletwrapper won't work, as
Remy stated.
  

Comment 25 Jonathan Leech 2005-09-23 19:26:14 UTC
Another thing that currently compunds this issue is the fact that the zip file
format used for .war files ignores timezone on datestamps.  

So for example, my file times are mountain standard and my server is in GMT.  If
I make a change to the .jsp after it is accessed on the server, unless it is 7
or 8 hours after, it won't take effect.  And if the time change is in the
reverse direction, the .jsp will be recompiled continuously for the duration of
the difference.
Comment 26 Jonathan Leech 2005-09-23 22:19:25 UTC
I took my Tomcat out of development mode, and verified this issue exists there
as well.  development=false uses the same, broken isOutDated check.

> On access compilation and its friend the development mode - which you are using
> or you would not have this "issue" - should not be used in production 
Comment 27 Tom Anderson 2005-10-05 03:05:01 UTC
> On access compilation and its friend the development mode - which you are using
> or you would not have this "issue" - should not be used in production

By "access compilation" are you referring to the development="true" mode which causes recompilation 
on every access?

We rely on our JSP pages to be compiled when the date changes and the only *reliable* way to do this is 
to touch *all* JSP files whenever we change something.   I suspect it is because of the problem Jonathon 
has pointed out.  Unfortunately, this means the webapp is very slow after each deployment.  If this 
problem won't be fixed, what is the recommended way to avoid recompiling all pages in a production 
environment?
Comment 28 Jonathan Leech 2005-10-05 17:16:56 UTC
Created attachment 16599 [details]
rebuilt jasper-compiler.jar

Here is a rebuilt jasper-compiler.jar that incorporates my proposed patch.  For
anyone who needs a fix and doesn't want to download, patch, and rebuild.
Comment 29 Tom Anderson 2005-10-19 22:59:40 UTC
We got bit by this bug again today!   How can I impress on the developers the seriousness of this issue?

We need to touch every JSP file when we deploy a webapp because we cannot trust that Tomcat will 
recompile the things that need to be.   This causes large delays to the end users that are unlucky enough 
to hit the website first.

If you revert your JSP files to an older branch, you also have to remember to touch them (many CM 
systems revert the dates to the older version which still pass the < comparison).

I think Jonathon's fix will address every realistic scenario.    Please use his patch!
Comment 30 Fabien Toral 2005-11-16 15:42:43 UTC
(In reply to comment #28)
> Created an attachment (id=16599) [edit]
> rebuilt jasper-compiler.jar
> 
> Here is a rebuilt jasper-compiler.jar that incorporates my proposed patch.  For
> anyone who needs a fix and doesn't want to download, patch, and rebuild.

Hello Jonathan,

i've tried tu use your jasper-compiler.jar into TC 5.5.12, because we have a
problem to make Tomcat reload and compile modified JSPs on fly, but an exception
is thrown when Jasper try to compile the JSP.

i haven't try to patch an rebuilt yet.

Here is the root cause of the stack trace, it seem like an import from an
Eclipse library :

java.lang.NoSuchMethodError:
org.eclipse.jdt.internal.compiler.env.NameEnvironmentAnswer.<init>(Lorg/eclipse/jdt/internal/compiler/env/IBinaryType;)V
	org.apache.jasper.compiler.JDTCompiler$1.findType(JDTCompiler.java:214)
	org.apache.jasper.compiler.JDTCompiler$1.findType(JDTCompiler.java:183)
	org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment.askForType(LookupEnvironment.java:119)
	org.eclipse.jdt.internal.compiler.lookup.PackageBinding.getTypeOrPackage(PackageBinding.java:178)
	org.eclipse.jdt.internal.compiler.lookup.Scope.getPackage(Scope.java:2111)
	org.eclipse.jdt.internal.compiler.ast.QualifiedTypeReference.getTypeBinding(QualifiedTypeReference.java:62)
	org.eclipse.jdt.internal.compiler.ast.TypeReference.resolveType(TypeReference.java:141)
	org.eclipse.jdt.internal.compiler.ast.TypeReference.resolveSuperType(TypeReference.java:104)
	org.eclipse.jdt.internal.compiler.lookup.ClassScope.findSupertype(ClassScope.java:1088)
	org.eclipse.jdt.internal.compiler.lookup.ClassScope.connectSuperclass(ClassScope.java:755)
	org.eclipse.jdt.internal.compiler.lookup.ClassScope.connectTypeHierarchy(ClassScope.java:927)
	org.eclipse.jdt.internal.compiler.lookup.CompilationUnitScope.connectTypeHierarchy(CompilationUnitScope.java:254)
	org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment.completeTypeBindings(LookupEnvironment.java:195)
	org.eclipse.jdt.internal.compiler.Compiler.beginToCompile(Compiler.java:301)
	org.eclipse.jdt.internal.compiler.Compiler.compile(Compiler.java:315)
	org.apache.jasper.compiler.JDTCompiler.generateClass(JDTCompiler.java:387)
	org.apache.jasper.compiler.Compiler.compile(Compiler.java:288)
	org.apache.jasper.compiler.Compiler.compile(Compiler.java:267)
	org.apache.jasper.compiler.Compiler.compile(Compiler.java:255)
	org.apache.jasper.JspCompilationContext.compile(JspCompilationContext.java:557)
	org.apache.jasper.servlet.JspServletWrapper.service(JspServletWrapper.java:293)
	org.apache.jasper.servlet.JspServlet.serviceJspFile(JspServlet.java:291)
	org.apache.jasper.servlet.JspServlet.service(JspServlet.java:241)
	javax.servlet.http.HttpServlet.service(HttpServlet.java:802)
Comment 31 Jonathan Leech 2005-11-16 17:05:10 UTC
Fabien,

The jasper-compiler.jar I built was against 5.5.9, I think that's the problem. 
It should work just fine if you patch and rebuild.

-Jonathan

(In reply to comment #30)
> (In reply to comment #28)
> > Created an attachment (id=16599) [edit] [edit]
> > rebuilt jasper-compiler.jar
> > 
> > Here is a rebuilt jasper-compiler.jar that incorporates my proposed patch.  For
> > anyone who needs a fix and doesn't want to download, patch, and rebuild.
> 
> Hello Jonathan,
> 
> i've tried tu use your jasper-compiler.jar into TC 5.5.12, because we have a
> problem to make Tomcat reload and compile modified JSPs on fly, but an exception
> is thrown when Jasper try to compile the JSP.
> 
> i haven't try to patch an rebuilt yet.
> 
> Here is the root cause of the stack trace, it seem like an import from an
> Eclipse library :
> 
> java.lang.NoSuchMethodError:
>
org.eclipse.jdt.internal.compiler.env.NameEnvironmentAnswer.<init>(Lorg/eclipse/jdt/internal/compiler/env/IBinaryType;)V
> 	org.apache.jasper.compiler.JDTCompiler$1.findType(JDTCompiler.java:214)
> 	org.apache.jasper.compiler.JDTCompiler$1.findType(JDTCompiler.java:183)
> 
org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment.askForType(LookupEnvironment.java:119)
> 
org.eclipse.jdt.internal.compiler.lookup.PackageBinding.getTypeOrPackage(PackageBinding.java:178)
> 	org.eclipse.jdt.internal.compiler.lookup.Scope.getPackage(Scope.java:2111)
> 
org.eclipse.jdt.internal.compiler.ast.QualifiedTypeReference.getTypeBinding(QualifiedTypeReference.java:62)
> 
org.eclipse.jdt.internal.compiler.ast.TypeReference.resolveType(TypeReference.java:141)
> 
org.eclipse.jdt.internal.compiler.ast.TypeReference.resolveSuperType(TypeReference.java:104)
> 
org.eclipse.jdt.internal.compiler.lookup.ClassScope.findSupertype(ClassScope.java:1088)
> 
org.eclipse.jdt.internal.compiler.lookup.ClassScope.connectSuperclass(ClassScope.java:755)
> 
org.eclipse.jdt.internal.compiler.lookup.ClassScope.connectTypeHierarchy(ClassScope.java:927)
> 
org.eclipse.jdt.internal.compiler.lookup.CompilationUnitScope.connectTypeHierarchy(CompilationUnitScope.java:254)
> 
org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment.completeTypeBindings(LookupEnvironment.java:195)
> 	org.eclipse.jdt.internal.compiler.Compiler.beginToCompile(Compiler.java:301)
> 	org.eclipse.jdt.internal.compiler.Compiler.compile(Compiler.java:315)
> 	org.apache.jasper.compiler.JDTCompiler.generateClass(JDTCompiler.java:387)
> 	org.apache.jasper.compiler.Compiler.compile(Compiler.java:288)
> 	org.apache.jasper.compiler.Compiler.compile(Compiler.java:267)
> 	org.apache.jasper.compiler.Compiler.compile(Compiler.java:255)
> 	org.apache.jasper.JspCompilationContext.compile(JspCompilationContext.java:557)
> 	org.apache.jasper.servlet.JspServletWrapper.service(JspServletWrapper.java:293)
> 	org.apache.jasper.servlet.JspServlet.serviceJspFile(JspServlet.java:291)
> 	org.apache.jasper.servlet.JspServlet.service(JspServlet.java:241)
> 	javax.servlet.http.HttpServlet.service(HttpServlet.java:802)

Comment 32 Tom Anderson 2006-03-21 23:45:34 UTC
I think the patch files are backwards (show how to remove the fix) but that doesn't make this bug less 
important.   We just got bit by this thing again... every couple of month at my company.   It's time to bite 
the bullet and make a one-off version.   Is it ever going to be fixed?
Comment 33 Jonathan Leech 2006-03-22 00:22:05 UTC
(In reply to comment #32)
Sorry for getting the patch backwards.  Not that it matters, it will probably 
never be incorporated any way, due to the co-location of a certain person's 
head with his ass.
Do the votes matter?  I am the only one voting for this, perhaps if a few more 
people used their votes it would show up on a list somewhere, causing some 
action to be taken. 
Other than patching Tomcat, here's what I recommend, in order of preference:
1) Don't use Tomcat.
2) Don't use JSPs.
4) Precompile your JSPs as part of your build process.
Comment 34 Tom Anderson 2006-03-22 03:29:40 UTC
Jonathoan, I was about to try using your patch and noticed a problem.   You are using the date of the 
JSP file to stamp the class file but the isOutDated() method is also taking into account the times of the 
included JSP files.   This can cause a JSP file to be re-compiled on every hit if it includes a file that has a 
later date. 

For example, if a.jsp is time-stamped 9:00 and it includes b.jsp which stamped 10:00, then isOutDated
() will always return true because the time of the included file is greater than 9:00.   Then when you 
assign the earlier time to the class file, this check will fail again the next time around.

I think a better solution is to set the time of the class file to the greatest time of the compiled file AND 
ALL of it's dependants.   In the above scenario, the class file would get a time stamp of 10:00 instead of 
9:00 and subsequent checks of isOutDated would return false as expected.   The only problem I can 
forsee is if someone updates the included file, THEN rolls back the original file.   That seems like a very 
obscure case and my solution is still better than the current solution which doesn't account for 
rollbacks at all (and also wouldn't handle the obscure case).

I will attempt a solution but will be on vacation soon so it may be a while before I'm able to post any 
patches. 
Comment 35 Jonathan Leech 2006-03-22 17:57:54 UTC
(In reply to comment #34)
Tom,  I see the problem with the dependants;  that is a huge problem with my patch.

I think it is worth solving this problem completely, in a manner similar to the
way Websphere does (see comment #24 and do the google search).  To do this, it
is necessary to compare the current and last timestamps of the jsp and all its
dependants.  Websphere persists this information in the comments of the
generated .java file.

I don't like the idea of putting the timestamps in the comments ala Websphere,
it will mean making changes in more places, and also I am not a fan of parsing text.

I am thinking along the lines of creating a seperate file which could be as
simple as a serialized hashmap containing the URLs as keys and the timestamps as
values.

Thoughts?
Comment 36 Scott Johnson 2006-03-22 18:54:41 UTC
Jonathan, just to clairfy:  WebSphere doesn't store the timestamp information 
in comments - what is stored in comments is informational data that can be 
used to help debug problems.
The timestamps used by the outdated checks are stored in the generated 
classfile as part of the _jspx_dependants List.  If you were to look at the 
generated .java source you would see, for example:

  private static String[] _jspx_dependants;
  static {
    _jspx_dependants = new String[2];
    _jspx_dependants[0] = "/Banner.jsp^1082410708000^Mon Apr 19 17:38:28 EDT 
2004";
    _jspx_dependants[1] = "/Footer.jsp^1077657462000^Tue Feb 24 16:17:42 EST 
2004";
  }

The timestamp simply follows a dependent's path information.

The data in comments the I referred to in post #24 is more this sort of thing:

e:/mytempdir/x.ear/y.war/WEB-INF/classes/_ibmjsp/_jsp1.java was generated @ 
Thu Mar 16 14:03:16 EST 2006
IBM WebSphere Application Server - ND, 6.1.0.0
    Build Number: v0611.54
    Build Date: 3/16/06

********************************************************
The JSP engine configuration parameters were set as follows:

classDebugInfo =              [false]
debugEnabled =                [false]
deprecation =                 [false]
compileWithAssert =           [false]
etc. etc. etc.

Hope this helps.
Comment 37 Jonathan Leech 2006-03-23 01:02:34 UTC
(In reply to comment #36)
Scott, thanks! That helps a whole lot.  I hadn't considered storing the
lastModified times of the dependencies as member data of the servlet.  It is the
ideal place.  In fact, Tomcat already stores the dependency list there.
I am currently testing a fix which stores the dependant lastModified times
there, as well as the lastModified time of the .jsp (rather than modify the
timestamp of the .class and .java files).  The comparisons made are all !=
instead of > .
I will post a new patch once I've tested my changes more thorougly.


Comment 38 Scott Johnson 2006-03-23 01:44:36 UTC
Jonathan, I have one caveat about using the servlet member data for the 
storing the timestamp of the JSP itself and using it for outdated check 
instead of using the JSP source and class file timestamps.  The problem is 
that the servlet class has to be loaded in order to retrieve this data.  
Therefore, you wouldn't know a servlet class file was outdated until you'd 
already loaded it once (first request). We decided that this 'lag' was ok for 
dependency checking (which by the way is turned off by default in WebSphere) 
but did not like this lag for the top-level JSP reloading.  As I said, just a 
caveat.
Comment 39 Jonathan Leech 2006-03-23 02:59:44 UTC
(In reply to comment #38)
Scott, I think I understand your point, but not fully.  Is the lag you refer to
the lag of loading the class twice vs. once, in the case of the .jsp being
outdated and its class not already loaded?  
As I understand it, if either the .jsp is not outdated, or the class is already
loaded, there would be no extra lag.  Is that correct?  I think I might be
missing something, or perhaps Tomcat works differently from Websphere.  Or are
there situations other than an incoming http request that trigger the outdated
check?
Comment 40 Scott Johnson 2006-03-23 13:35:29 UTC
(In reply to comment #39)
What you wrote is totally correct.  I don't want to confuse the issue so I'll 
shut up now.  :)
Comment 41 Jonathan Leech 2006-03-23 20:10:54 UTC
Created attachment 17955 [details]
Compiler.java diff
Comment 42 Jonathan Leech 2006-03-23 20:11:23 UTC
Created attachment 17956 [details]
Generator.java diff
Comment 43 Jonathan Leech 2006-03-23 20:11:53 UTC
Created attachment 17957 [details]
JspSourceDependent.java diff
Comment 44 Jonathan Leech 2006-03-23 20:12:19 UTC
Created attachment 17958 [details]
JspServletWrapper.java diff
Comment 45 Jonathan Leech 2006-03-23 20:15:53 UTC
I uploaded the diffs for the 4 changed files.  This fixes the problem completely
and addresses any and all concerns brought forth in the comments so far.  What
happens next?
Comment 46 Jonathan Leech 2006-04-06 18:07:25 UTC
Anyone?  Bueller?
Comment 47 Jonathan Leech 2006-04-27 21:35:34 UTC
http://jira.jboss.com/jira/browse/JBAS-3081?page=all
JBoss was one of the last standouts, it was relying on Jasper to decide to
recompile or not.  No more.  Now everything gets recompiled on every redeploy. 
At least we can count on the correctness of its behavior now.  

Too bad for the performance that according to some comments here was of such
paramount concern.  

Its also too bad that even with patches charitably submitted, that bugs can't or
won't be fixed in Jasper.

Comment 48 Remy Maucherat 2006-04-28 00:44:54 UTC
(In reply to comment #47)
> Its also too bad that even with patches charitably submitted, that bugs can't or
> won't be fixed in Jasper.

As I said earlier, I don't think fixing the edge cases is worth adding the
related complexity. It's very simple. As for JBoss "fixing" it, Tomcat "fixes"
it too: you simply need to undeploy/redeploy webapps, and/or add some listener
to clean up the work directory, which is trivial to do. If that's all you want
to achieve, why did you focus on a complex patch to Jasper ?

BTW, feel free to post more useless rants, esp in conjunction with Gili, I enjoy
them :)
Comment 49 Jonathan Leech 2006-04-28 14:05:26 UTC
(In reply to comment #48)
> As I said earlier, I don't think fixing the edge cases is worth adding the
> related complexity. It's very simple. As for JBoss "fixing" it, Tomcat "fixes"
> it too: you simply need to undeploy/redeploy webapps, and/or add some listener
> to clean up the work directory, which is trivial to do. If that's all you want
> to achieve, why did you focus on a complex patch to Jasper ?

The problem isn't isolated to an "edge" case, it affects the standard way apps
are deployed.  Thus the numerous other people who have encountered it.  As for
my patch being "complex" that's just not true. I added 2 fields to the generated
servlet class and updated the isOutDated logic to use them.  It took all of an
hour to code and test.  I can't believe you're asking me why I wanted to fix
something that is broken rather than bandaid it.

Comment 50 Michał Borowiecki 2006-05-03 20:54:05 UTC
> The problem isn't isolated to an "edge" case, it affects the standard way apps
> are deployed.  Thus the numerous other people who have encountered it. 

I certainly agree. I encountered this issue multiple times. I took quite some
time and flustration to discover this bug. 
The workaround I currently use is deploying exploded .war contents and setting
scp not to preserve timestamps, effectively forcing all copied JSP files to be
recompiled.
I think the bug is obviously serious and is definately not isolated to an "edge"
case. 
I'm glad someone is trying to fix it.
Comment 51 Jonathan Leech 2006-05-03 21:00:09 UTC
(In reply to comment #50)
> > The problem isn't isolated to an "edge" case, it affects the standard way apps
> > are deployed.  Thus the numerous other people who have encountered it. 
> 
> I certainly agree. I encountered this issue multiple times. I took quite some
> time and flustration to discover this bug. 
> The workaround I currently use is deploying exploded .war contents and setting
> scp not to preserve timestamps, effectively forcing all copied JSP files to be
> recompiled.
> I think the bug is obviously serious and is definately not isolated to an "edge"
> case. 
> I'm glad someone is trying to fix it.

I've fixed it, getting the fix incorporated into the codebase appears to be the
impossible part.  Feel free to use the code from my patch and give feedback if
you find any problems.
Comment 52 Michał Borowiecki 2006-05-06 14:04:38 UTC
(In reply to comment #51)
> I've fixed it, getting the fix incorporated into the codebase appears to be the
> impossible part.  Feel free to use the code from my patch and give feedback if
> you find any problems.

Thanks, but it seems we will be migrating to JBoss 4.0.4 when the GA version
comes out and it shouldn't be an issue anymore. I'm fortunate enough I don't
have to use Tomcat standalone.
Comment 53 Larry Prikockis 2006-05-11 20:38:26 UTC
(In reply to comment #51)

> I've fixed it, getting the fix incorporated into the codebase appears to be the
> impossible part.  Feel free to use the code from my patch and give feedback if
> you find any problems.

I just wasted a embarrasing amount of time trying to figure out why one of our
developers was having problems seeing her changes 'live' on the test machine
after changing a JSP file.  I'd assumed that since I had Tomcat running in
Development mode, it was going to recompile the page when it changed.... 

I'm glad to see that the seriousness of this bug has been recognized, and a
patch developed... even if it doesn't seem to have the blessing of some of the
developers.  The patch certainly saved me a lot of hassle.  Thanks!!
Comment 54 Mark Thomas 2006-09-06 01:46:27 UTC
*** Bug 40420 has been marked as a duplicate of this bug. ***
Comment 55 Ron Bodkin 2006-09-06 23:51:59 UTC
I think this is a significant problem when you really have different
organizations building and deploying, e.g., for publicly distributed
applications. A common timeline would be:
January - release 1.0 of app
March - JSP in app is updated
April - release 1.1 of app frozen for QA
May - user downloads and installs 1.0 
June - user downloads and installs 1.1

In this case, Jasper will view the compiled date of the 1.0 JSP as May and view
it as newer than the change date of the 1.1 JSP. I gather that the "best
practice" for building a web app for use with Tomcat would be to touch all the
JSP's in your web app in your release process so you minimize the risk (although
if someone downloads 1.0 after you released 1.1 they can expect a lot of 500
errors from NoSuchMethodError)

I believe Jasper really should remove cached JSP's from the work directory when
an app is undeployed (or redeployed). I think this is common, important, and
quite different than the rather obscure case of rolling back an older version
from version control. This would also avoid a performance hit in checking out of
date on JSP's and wouldn't surprise people (I wouldn't expect JSP's to be cached
after redeploying an app, indeed I think it's surprising behavior!)

One of the users of our Web app just hit this issue today:
http://www.glassbox.com/forum/forum/addpost?parent=235 and with a little
googling you can see others e.g.,
http://mail-archives.apache.org/mod_mbox/tomcat-users/200512.mbox/%3C43A0096A.3060200@mkodo.com%3E

Comment 56 Jonathan Leech 2006-09-06 23:59:18 UTC
For the next release of your software, I would register an error handler that 
catches this error, and sends an email with the contents to the tomcat-dev 
mailing list.

(In reply to comment #55)
> if someone downloads 1.0 after you released 1.1 they can expect a lot of 500
> errors from NoSuchMethodError)

Comment 57 Mark Thomas 2006-09-07 00:11:13 UTC
Why not just distribute your app with pre-compilied JSPs and avoid all these
problems?
Comment 58 Ron Bodkin 2006-09-07 00:18:52 UTC
Our app needs to be portable to a variety of Servlet containers (and for
different versions), so we can't precompile for any one server.
Comment 59 Darryl Miles 2006-09-07 05:54:51 UTC
(In reply to comment #55)
> I believe Jasper really should remove cached JSP's from the work directory when
> an app is undeployed (or redeployed). I think this is common, important, and
> quite different than the rather obscure case of rolling back an older version
> from version control. This would also avoid a performance hit in checking out of
> date on JSP's and wouldn't surprise people (I wouldn't expect JSP's to be cached
> after redeploying an app, indeed I think it's surprising behavior!)

This is a very good point that I would agree with.  It is not upto any external
tools to effectively manage the "work/" directory for Tomcat.  This directory
should be self-managing and be implemented on the side of caution, the caching
of JSP pages is a benifit not a right.

I think the following new rule would work:
 * The work/ directory is only cleaned of unused contexts when a web-app is
undeployed (while the container is running, aka hot-undeploy) or found to no
longer exist after all configuration parsing has been done at container startup.

Although it is somewhat difficult to manage web-app upgrades taking place when
the container is shutdown.  Which I'd say was a pretty common event.

One way around that situation would be to detect a web-app update has taken
place.  The simplest for TC (and the sys-admin) way I can think of, is for TC to
remember the exact timestamp on the WEB-INF/web.xml file which the pre-compiled
pages relate to.  Make it create an empty file and touch up the timestamp to
match the real web.xml as work/web.xml.timestamp.

The sys-admin must then only touch the WEB-INF/web.xml (when he upgrades his
web-app while the container is stopped).  When TC boots up again it detects the
timestamp is not equal and presumes the web-app was changed also, this causes a
flush of the work/ for that context.

The idea being this approach would be a whole lot simpler than re-validating the
entire work/ cache from the source JSPs during all webapp deployments.


It would be nice to have re-validation maybe that could be implemented using
magic .java file comments in the japser output "// Jasper-JSP-Prerequisite:
foobar/test.jsp 72383828372000" where both the top level source and all included
fragments are listed with their Epoch millis for timestamp.  Then the process
would be to recurse the work/org/apache/ tree, reading all the .java and
performing a simple stat() on the source files.  This could be done in the
background with live requests taking priority to be checked on first access
after deployment.

When work/* file that have been sucessfully revalidated (or deleted/recreated)
have their timestamp updated, so that it is possible for any thread to know if a
re-validation is required on a page, since the timestamp will be older than the
deployment time of the web-app context.

This would have no longterm JVM impact that loading classes might have, we can
put what we like in the .java file and access it easily.
Comment 60 Jonathan Leech 2006-09-07 15:18:15 UTC
Daryl,  see comments #35 - #45 and the 4 patch files I posted in the 
Attachment section.  The patch I submitted in March completely fixes this 
problem in a manner similar to your suggestion.

The patch works by storing the timestamp of the .jsp and all its 
dependent .jsps as member data in the compiled servlet.  The isOutDated() 
method was modified to compare the timestamp of the .jsp against the added 
timestamps using != instead of >.

(In reply to comment #59)
> It would be nice to have re-validation maybe that could be implemented using
> magic .java file comments in the japser output "// Jasper-JSP-Prerequisite:
> foobar/test.jsp 72383828372000" where both the top level source and all 
included
> fragments are listed with their Epoch millis for timestamp.  Then the process
> would be to recurse the work/org/apache/ tree, reading all the .java and
> performing a simple stat() on the source files.  This could be done in the
> background with live requests taking priority to be checked on first access
> after deployment.
Comment 61 Darryl Miles 2006-09-08 05:29:02 UTC
What are the side-effects of revalidating the entire tree ?  Does it cause all
class files to be loaded or can the revalidation occur without having any
lasting overheads (like increased memory consumption and slower revalidation
process due to parsing of more complex .class data).

My method only seeks to delete stale work/ .java and .class files during web-app
deployment.

It does not seek to recreate and load them, that can be left to moment of first
use (although it would natually lead on to facilitating an automatic recreation
 function).

By opening the .java file and looking for a magic comment and closing the file
again, there is no lasting overhead.  Since we never loaded the class.  Which is
just great for a revalidation pass during deployment.

I'm a believer there should be a configuration mode of TC which is watertight,
such that no amount of abuse will make the things fail in a way that bites you.
 The work/ directory is a nice speedup but the implementation is more a hack
than an optimization, since it clearly breaks down in situations you wouldn't
expect.

This bug/problem hits developers a lot more than production upgrades.
Comment 62 Jonathan Leech 2006-09-08 15:43:30 UTC
My patch doesn't change the overall strategy for invoking the isOutDated() 
check, as you are suggesting. The isOutDated() check does load the class, as 
it did prior to my patch. Revalidating the entire tree would cause all the 
class files to be loaded, which would be bad. However, with the isOutDated() 
method fixed, I see no reason to revalidate the entire tree. 

(In reply to comment #61)
> What are the side-effects of revalidating the entire tree ?  Does it cause 
all
> class files to be loaded or can the revalidation occur without having any
> lasting overheads (like increased memory consumption and slower revalidation
> process due to parsing of more complex .class data).
> My method only seeks to delete stale work/ .java and .class files during web-
app
> deployment.
> It does not seek to recreate and load them, that can be left to moment of 
first
> use (although it would natually lead on to facilitating an automatic 
recreation
>  function).
> By opening the .java file and looking for a magic comment and closing the 
file
> again, there is no lasting overhead.  Since we never loaded the class.  
Which is
> just great for a revalidation pass during deployment.
> I'm a believer there should be a configuration mode of TC which is 
watertight,
> such that no amount of abuse will make the things fail in a way that bites 
you.
>  The work/ directory is a nice speedup but the implementation is more a hack
> than an optimization, since it clearly breaks down in situations you wouldn't
> expect.
> This bug/problem hits developers a lot more than production upgrades.

Comment 63 Yoav Shapira 2006-12-26 17:13:24 UTC
(In reply to comment #62)
Darryl's last comments aside on changing the management of the entire work tree,
I want to ask Jonathan and anyone who's used his patches: have they been stable
and OK?  Have there been any modifications needed to them?  If not, i.e. if
they've been stable, I'm tempted to add them to the 5.5 tree.

Comment 64 Jonathan Leech 2006-12-28 06:42:57 UTC
I haven't had to make any additional changes to the code in the patch.  I have 
only used the patch in conjunction with JBoss 4.0.2.  I have been using the 
code in development and production since I posted it.

(In reply to comment #63)
> (In reply to comment #62)
> Darryl's last comments aside on changing the management of the entire work 
tree,
> I want to ask Jonathan and anyone who's used his patches: have they been 
stable
> and OK?  Have there been any modifications needed to them?  If not, i.e. if
> they've been stable, I'm tempted to add them to the 5.5 tree.

Comment 65 Luis Naver 2008-08-11 14:00:43 UTC
I just got bit by this bug and lost a good part of my day trying to locate it.  I have tomcat on my development machine (winXP) configured to UTC and the machine itself is set for PST. 

Here's the procedure

 * make a change to my source jsp
 * war it up
 * stop tomcat
 * delete the existing war and webapps folder
 * copy the new war into the webapps dir
 * start tomcat

The problem:
Tomcat will expand the war however the last modified date on the jsp file will be adjusted (incorrectly?) to 8 hrs earlier.  As the previous change was compiled less than 8hrs ago the jsp is not recompiled and the outdated .java and .class files remain in the work\Catalina\localhost directory.  The result is my changes are not reflected.

For now I will delete the work directory at the same time I delete the old war file.  This way at least, as a developer, I know that what tomcat is serving is my most recent code.
Comment 66 Mark Thomas 2010-12-17 09:14:37 UTC
*** Bug 15417 has been marked as a duplicate of this bug. ***
Comment 67 Mark Thomas 2011-05-20 16:26:13 UTC
Created attachment 27040 [details]
Proposed patch for Tomcat 7

Having spent a little time on this, I am attaching a proposed patch for Tomcat 7. The patch breaks binary compatibility for compiled JSPs which I am not at all comfortable about. I have some ideas for a solution to that which I will be discussing on the dev list.

If this issue is addressed in Tomcat 7, I don't see the fix being back-ported to earlier versions.
Comment 68 Mark Thomas 2011-05-28 18:55:51 UTC
Feedback on proposed TC7 patch:
http://tomcat.markmail.org/thread/mbjdpr4bvw6gzx62
Comment 69 Mark Thomas 2011-06-20 18:03:09 UTC
This has been fixed in 7.0.x and will be included in 7.0.17 onwards.

The fix was fairly invasive so it will not be back-ported to 6.0.x or 5.5.x.