Thread: Re: [Webwork-devel] Webwork 1.0.3 / CVS taglib totally broken?
Brought to you by:
baldree,
rickardoberg
Menu
▾
▴
webwork-devel
|
From: <ma...@sm...> - 2002-05-23 12:21:05
|
For WebLogic, using release is the preferred method. I disagree with your blanket statement. I fixed the release order in the latest CVS and in the branch. I'm told that it is working correctly on Orion but I'm waiting for official confirmation. I would appreciate if you could check it with the latest CVS and let me know. On Thu, 23 May 2002, Mike Cannon-Brookes wrote > > Yes - release() is the bane of tag developers. > > The simple rule is basically - if you're using release, you're doing things > wrong! > > -mike > > On 23/5/02 4:28 PM, "Heng Sin Low" (low...@ya...) penned the words: > > > Ha, finally u got what I mean :) > > > > Yes, doEndTag should be the right place. I remember > > Matt have done a massive check in recently that move > > all this resetting stuff to the release() method. > > > > Regards, > > Low > > > >> I'm trying to what out what changes have occurred > > recently, but the taglib > > in CVS is currently broken and not backward > > compatible. > > > > > > __________________________________________________ > > Do You Yahoo!? > > LAUNCH - Your Yahoo! Music Experience > > http://launch.yahoo.com > > > > _______________________________________________________________ > > > > Don't miss the 2002 Sprint PCS Application Developer's Conference > > August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm > > > > _______________________________________________ > > Webwork-devel mailing list > > Web...@li... > > https://lists.sourceforge.net/lists/listinfo/webwork-devel > > > _______________________________________________________________ > > Don't miss the 2002 Sprint PCS Application Developer's Conference > August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm > > _______________________________________________ > Webwork-devel mailing list > Web...@li... > https://lists.sourceforge.net/lists/listinfo/webwork-devel |
|
From: <ma...@sm...> - 2002-05-23 12:22:38
|
Again, could you check this with the updated CVS and let me know. On Thu, 23 May 2002, Mike Cannon-Brookes wrote > > Ok - I have found the exact commit that fucked everything up. > > http://cvs.sourceforge.net/cgi-bin/viewcvs.cgi/webwork/webwork/src/main/webw > ork/view/taglib/ui/ComponentTag.java.diff?r1=1.1&r2=1.2 > > BAD BAD BAD BAD BAD :) > (apologies - just venting after spending 4 hours tracking this bastard down > ;)) > > In my local copy of CVS I've removed the release() method of the > ComponentTag, and put back the finally clause to the end of the doEndTag() > method like so: > > finally > { > getStack().popValue(); > params = new HashMap(); > } > > This appears to make everything work again (I haven't had a chance to fully > test it - but my test case now works). > > Can we commit this to CVS and put out 1.0.4? > > Cheers, > Mike > > PS I can't believe this hasn't been caught in months. Am we the only WW > users using the UI tags with Orion and Resin? :) This would have broken > pretty much all uses of the UI tags! > > > Mike Cannon-Brookes > mi...@at... > > ATLASSIAN - Your J2EE Expert Partner > -------------------------------------------------------- > > Brilliant Software - http://www.atlassian.com/software > > Legendary Services - http://www.atlassian.com/support > > On 23/5/02 4:23 PM, "Mike Cannon-Brookes" (mi...@at...) penned the > words: > > > I'm getting further in tracking this down. > > > > It broke between 1.0.1 (which works fine) and 1.0.2 (which doesn't work). > > > > I'll see if I can find the exact commits and create a patch for the current > > CVS (to save rollbacks). > > > > -mike > > > > On 23/5/02 4:07 PM, "Mike Cannon-Brookes" (mi...@at...) penned the > > words: > > > >> I'm trying to what out what changes have occurred recently, but the taglib > >> in CVS is currently broken and not backward compatible. > >> > >> I think it's to do with the recent release() movements etc. > >> > >> Values in correct containers (tested in both Orion and Resin) no longer get > >> reset! > >> > >> Ie > >> > >> textfield: text = "foo" > >> ... Jsp ... > >> textfield: text = "foo" > >> (this should be "" as the value in the stack is null, but the tag still has > >> the old value because it is no longer cleared) > >> > >> I think this manifests itself in a number of places. > >> > >> I'm betting (investigating the exact cause now) that this is because you're > >> relying on release() being called everytime the tag is executed. > >> > >> AFAIK (from memory I submitted a fix for this a while ago and the same error > >> has crept in again) this is NOT what the specification specifies (I haven't > >> got it with me) - this is how poor containers (Tomcrap) implement it. > >> > >> release() is called by the container when it pleases, to release resources > >> the tag is holding (the only time it must be called is before a GC I seem to > >> recall). > >> > >> If you want to _reset_ values you should do this at the end of doEndTag(). > >> > >> I'll try to fix this and submit patches but I suggest a rollback to previous > >> source (as it would be easier!). Any idea why this was changed? It worked > >> perfectly for me before. > >> > >> Maybe I should have just paid more attention on the devel list, or maybe I'm > >> just v. tired ;) > >> > >> -mike > >> > >> > >> Mike Cannon-Brookes > >> mi...@at... > >> > >> ATLASSIAN - Your J2EE Expert Partner > >> -------------------------------------------------------- > >>> Brilliant Software - http://www.atlassian.com/software > >>> Legendary Services - http://www.atlassian.com/support > >> > >> > >> _______________________________________________________________ > >> > >> Don't miss the 2002 Sprint PCS Application Developer's Conference > >> August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm > >> > >> _______________________________________________ > >> Webwork-devel mailing list > >> Web...@li... > >> https://lists.sourceforge.net/lists/listinfo/webwork-devel > > > > > > _______________________________________________________________ > > > > Don't miss the 2002 Sprint PCS Application Developer's Conference > > August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm > > > > _______________________________________________ > > Webwork-devel mailing list > > Web...@li... > > https://lists.sourceforge.net/lists/listinfo/webwork-devel > > > _______________________________________________________________ > > Don't miss the 2002 Sprint PCS Application Developer's Conference > August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm > > _______________________________________________ > Webwork-devel mailing list > Web...@li... > https://lists.sourceforge.net/lists/listinfo/webwork-devel |
|
From: <ma...@sm...> - 2002-05-23 13:16:55
|
I moved the popValue() into doEndTag() and tested it with Resin and WL 7.0. Give this a try and see if your problems are fixed. -Matt On Thu, 23 May 2002, Mike Cannon-Brookes wrote > > Ok - I thought that fixed it but that is just the tip of the iceberg. > > Pretty much every tag now 'resets' itself in the release() method - argh! > > I'll try to prepare a fill set of diffs. > > -mike > > On 23/5/02 4:35 PM, "Mike Cannon-Brookes" (mi...@at...) penned the > words: > > > Ok - I have found the exact commit that fucked everything up. > > > > http://cvs.sourceforge.net/cgi-bin/viewcvs.cgi/webwork/webwork/src/main/webw > > ork/view/taglib/ui/ComponentTag.java.diff?r1=1.1&r2=1.2 > > > > BAD BAD BAD BAD BAD :) > > (apologies - just venting after spending 4 hours tracking this bastard down > > ;)) > > > > In my local copy of CVS I've removed the release() method of the > > ComponentTag, and put back the finally clause to the end of the doEndTag() > > method like so: > > > > finally > > { > > getStack().popValue(); > > params = new HashMap(); > > } > > > > This appears to make everything work again (I haven't had a chance to fully > > test it - but my test case now works). > > > > Can we commit this to CVS and put out 1.0.4? > > > > Cheers, > > Mike > > > > PS I can't believe this hasn't been caught in months. Am we the only WW > > users using the UI tags with Orion and Resin? :) This would have broken > > pretty much all uses of the UI tags! > > > > > > Mike Cannon-Brookes > > mi...@at... > > > > ATLASSIAN - Your J2EE Expert Partner > > -------------------------------------------------------- > >> Brilliant Software - http://www.atlassian.com/software > >> Legendary Services - http://www.atlassian.com/support > > > > On 23/5/02 4:23 PM, "Mike Cannon-Brookes" (mi...@at...) penned the > > words: > > > >> I'm getting further in tracking this down. > >> > >> It broke between 1.0.1 (which works fine) and 1.0.2 (which doesn't work). > >> > >> I'll see if I can find the exact commits and create a patch for the current > >> CVS (to save rollbacks). > >> > >> -mike > >> > >> On 23/5/02 4:07 PM, "Mike Cannon-Brookes" (mi...@at...) penned the > >> words: > >> > >>> I'm trying to what out what changes have occurred recently, but the taglib > >>> in CVS is currently broken and not backward compatible. > >>> > >>> I think it's to do with the recent release() movements etc. > >>> > >>> Values in correct containers (tested in both Orion and Resin) no longer get > >>> reset! > >>> > >>> Ie > >>> > >>> textfield: text = "foo" > >>> ... Jsp ... > >>> textfield: text = "foo" > >>> (this should be "" as the value in the stack is null, but the tag still has > >>> the old value because it is no longer cleared) > >>> > >>> I think this manifests itself in a number of places. > >>> > >>> I'm betting (investigating the exact cause now) that this is because you're > >>> relying on release() being called everytime the tag is executed. > >>> > >>> AFAIK (from memory I submitted a fix for this a while ago and the same error > >>> has crept in again) this is NOT what the specification specifies (I haven't > >>> got it with me) - this is how poor containers (Tomcrap) implement it. > >>> > >>> release() is called by the container when it pleases, to release resources > >>> the tag is holding (the only time it must be called is before a GC I seem to > >>> recall). > >>> > >>> If you want to _reset_ values you should do this at the end of doEndTag(). > >>> > >>> I'll try to fix this and submit patches but I suggest a rollback to previous > >>> source (as it would be easier!). Any idea why this was changed? It worked > >>> perfectly for me before. > >>> > >>> Maybe I should have just paid more attention on the devel list, or maybe I'm > >>> just v. tired ;) > >>> > >>> -mike > >>> > >>> > >>> Mike Cannon-Brookes > >>> mi...@at... > >>> > >>> ATLASSIAN - Your J2EE Expert Partner > >>> -------------------------------------------------------- > >>>> Brilliant Software - http://www.atlassian.com/software > >>>> Legendary Services - http://www.atlassian.com/support > >>> > >>> > >>> _______________________________________________________________ > >>> > >>> Don't miss the 2002 Sprint PCS Application Developer's Conference > >>> August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm > >>> > >>> _______________________________________________ > >>> Webwork-devel mailing list > >>> Web...@li... > >>> https://lists.sourceforge.net/lists/listinfo/webwork-devel > >> > >> > >> _______________________________________________________________ > >> > >> Don't miss the 2002 Sprint PCS Application Developer's Conference > >> August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm > >> > >> _______________________________________________ > >> Webwork-devel mailing list > >> Web...@li... > >> https://lists.sourceforge.net/lists/listinfo/webwork-devel > > > > > > _______________________________________________________________ > > > > Don't miss the 2002 Sprint PCS Application Developer's Conference > > August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm > > > > _______________________________________________ > > Webwork-devel mailing list > > Web...@li... > > https://lists.sourceforge.net/lists/listinfo/webwork-devel > > > _______________________________________________________________ > > Don't miss the 2002 Sprint PCS Application Developer's Conference > August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm > > _______________________________________________ > Webwork-devel mailing list > Web...@li... > https://lists.sourceforge.net/lists/listinfo/webwork-devel |
|
From: <ma...@sm...> - 2002-05-23 15:04:08
|
Alright. Thanks for the reply. We'll get this worked out and commented accordingly. Whatever the outcome, I'll check it against Resin, WL, and Tomcat. I'll leave you and others to check it against Orion. As soon as I get a thumbs up from the community, I'll release a patch 1.0.4. -Matt On Fri, 24 May 2002, Mike Cannon-Brookes wrote > > Matt, > > Thanks mate - I'll check the CVS in the morning and report back. I found a > bunch of changes I needed to make today to get it working, so I'll see how > CVS works for me and start from there again tomorrow (with more mental > energy). > > Re: release() - this is NOT the preferred method at all ;) > > Again (yes, I've checked now with the spec and the author of JSP Tags book) > release() is called by the container to release resources associated with > the tag. It can be called at ANY time the container wishes (you have no > guarantee of when it is called). The only restriction is that it must be > called before the tag instance is garbage collected. > > So - what you have is lazy programmers (WL, Tomcat are the two glaring > examples) who just call release() after every tag invocation - which leads > to people's misplaced belief that it MUST be called everytime. This is not > so! :) Fast containers (like Resin and Orion) pool tag instances and reuse > them (maximising speed), and call release() as few times as possible > (sometimes never). I'm sure it's harder to code it this way, but the end > result is speed. > > Net result: > > NEVER do anything you want to happen in release()! > > This is true across all the JSP tags in WW and other projects. If you want > logic to occur, or variables to be reset for each tag invocation, reset them > at the end of the doEndTag() method (either before returning, or as a > finally block). > > I'll check CVS in the morning - good work on the quick fixes though! > > Cheers, > Mike > > On 23/5/02 10:20 PM, "ma...@sm..." (ma...@sm...) penned the > words: > > > For WebLogic, using release is the preferred method. I > > disagree with your blanket statement. I fixed the > > release order in the latest CVS and in the branch. I'm > > told that it is working correctly on Orion but I'm > > waiting for official confirmation. I would appreciate > > if you could check it with the latest CVS and let me > > know. > > > > On Thu, 23 May 2002, Mike Cannon-Brookes wrote > > > >> > >> Yes - release() is the bane of tag developers. > >> > >> The simple rule is basically - if you're using > > release, you're doing things > >> wrong! > >> > >> -mike > >> > >> On 23/5/02 4:28 PM, "Heng Sin Low" > > (low...@ya...) penned the words: > >> > >>> Ha, finally u got what I mean :) > >>> > >>> Yes, doEndTag should be the right place. I remember > >>> Matt have done a massive check in recently that move > >>> all this resetting stuff to the release() method. > >>> > >>> Regards, > >>> Low > >>> > >>>> I'm trying to what out what changes have occurred > >>> recently, but the taglib > >>> in CVS is currently broken and not backward > >>> compatible. > >>> > >>> > >>> __________________________________________________ > >>> Do You Yahoo!? > >>> LAUNCH - Your Yahoo! Music Experience > >>> http://launch.yahoo.com > >>> > >>> > > _______________________________________________________________ > >>> > >>> Don't miss the 2002 Sprint PCS Application > > Developer's Conference > >>> August 25-28 in Las Vegas -- > > http://devcon.sprintpcs.com/adp/index.cfm > >>> > >>> _______________________________________________ > >>> Webwork-devel mailing list > >>> Web...@li... > >>> > > https://lists.sourceforge.net/lists/listinfo/webwork-devel > >> > >> > >> > > _______________________________________________________________ > >> > >> Don't miss the 2002 Sprint PCS Application > > Developer's Conference > >> August 25-28 in Las Vegas -- > > http://devcon.sprintpcs.com/adp/index.cfm > >> > >> _______________________________________________ > >> Webwork-devel mailing list > >> Web...@li... > >> > > https://lists.sourceforge.net/lists/listinfo/webwork-devel > > > > > > > > _______________________________________________________________ > > > > Don't miss the 2002 Sprint PCS Application Developer's Conference > > August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm > > > > _______________________________________________ > > Webwork-devel mailing list > > Web...@li... > > https://lists.sourceforge.net/lists/listinfo/webwork-devel |
|
From: Mike Cannon-B. <mi...@at...> - 2002-05-27 07:50:20
|
Ok - I've checked and from preliminary looks it seems like the ComponentTag is fixed in Orion (haven't tested Resin yet - will do) PropertyTag: I had to fix the PropertyTag again though. Simple fix: (can't check in myself but here's the details) - remove return statement that's not at the end of the doEndTag method) - call release() manually before return EVAL_PAGE; (see attachment) Again - the problem here is that data is reset in the release() method which is incorrect ;) I have a suggested fix for every tag - rename ALL the release() methods to reset() and call them manually at the end of the doEndTag() methods (or before every return EVAL_PAGE statement) as well. This keeps the 'reset' logic compartmentalised but at the same time makes sure that it will be called on ALL containers. I've done this in the attached file so you can see what I mean. TextUtil: Also I changed the default value of "escapeOuterQuotes" to true. Why would you ever want this to be false? (Having it false by default means for example that any formfield with "text" as a value only gets text which is v. dangerous) Can we change this in CVS? Why was escapeOuterQuotes ever needed? I'll yell again if I find any more specific problems - but I can see that the release() method does things in other tag (which I probably don't use) which is bad. Cheers, Mike PS More investigating (this time in Resin) since I wrote the above - there are now a lot of NPEs due to the new ValueStack stuff. Line 96 of ValueStack.java and line 403 of AbstractValueStack.java (both probably related to null values being looked up and not checked for before calling method on the value object) Mike Cannon-Brookes mi...@at... ATLASSIAN - Your J2EE Expert Partner -------------------------------------------------------- > Brilliant Software - http://www.atlassian.com/software > Legendary Services - http://www.atlassian.com/support On 24/5/02 1:03 AM, "ma...@sm..." (ma...@sm...) penned the words: > Alright. Thanks for the reply. We'll get this worked > out and commented accordingly. Whatever the outcome, > I'll check it against Resin, WL, and Tomcat. I'll leave > you and others to check it against Orion. As soon as I > get a thumbs up from the community, I'll release a > patch 1.0.4. > > -Matt > > On Fri, 24 May 2002, Mike Cannon-Brookes wrote > >> >> Matt, >> >> Thanks mate - I'll check the CVS in the morning and > report back. I found a >> bunch of changes I needed to make today to get it > working, so I'll see how >> CVS works for me and start from there again tomorrow > (with more mental >> energy). >> >> Re: release() - this is NOT the preferred method at > all ;) >> >> Again (yes, I've checked now with the spec and the > author of JSP Tags book) >> release() is called by the container to release > resources associated with >> the tag. It can be called at ANY time the container > wishes (you have no >> guarantee of when it is called). The only restriction > is that it must be >> called before the tag instance is garbage collected. >> >> So - what you have is lazy programmers (WL, Tomcat > are the two glaring >> examples) who just call release() after every tag > invocation - which leads >> to people's misplaced belief that it MUST be called > everytime. This is not >> so! :) Fast containers (like Resin and Orion) pool > tag instances and reuse >> them (maximising speed), and call release() as few > times as possible >> (sometimes never). I'm sure it's harder to code it > this way, but the end >> result is speed. >> >> Net result: >> >> NEVER do anything you want to happen in release()! >> >> This is true across all the JSP tags in WW and other > projects. If you want >> logic to occur, or variables to be reset for each tag > invocation, reset them >> at the end of the doEndTag() method (either before > returning, or as a >> finally block). >> >> I'll check CVS in the morning - good work on the > quick fixes though! >> >> Cheers, >> Mike >> >> On 23/5/02 10:20 PM, "ma...@sm..." > (ma...@sm...) penned the >> words: >> >>> For WebLogic, using release is the preferred > method. I >>> disagree with your blanket statement. I fixed the >>> release order in the latest CVS and in the branch. > I'm >>> told that it is working correctly on Orion but I'm >>> waiting for official confirmation. I would > appreciate >>> if you could check it with the latest CVS and let me >>> know. >>> >>> On Thu, 23 May 2002, Mike Cannon-Brookes wrote >>> >>>> >>>> Yes - release() is the bane of tag developers. >>>> >>>> The simple rule is basically - if you're using >>> release, you're doing things >>>> wrong! >>>> >>>> -mike >>>> >>>> On 23/5/02 4:28 PM, "Heng Sin Low" >>> (low...@ya...) penned the words: >>>> >>>>> Ha, finally u got what I mean :) >>>>> >>>>> Yes, doEndTag should be the right place. I > remember >>>>> Matt have done a massive check in recently that > move >>>>> all this resetting stuff to the release() method. >>>>> >>>>> Regards, >>>>> Low >>>>> >>>>>> I'm trying to what out what changes have occurred >>>>> recently, but the taglib >>>>> in CVS is currently broken and not backward >>>>> compatible. >>>>> >>>>> >>>>> __________________________________________________ >>>>> Do You Yahoo!? >>>>> LAUNCH - Your Yahoo! Music Experience >>>>> http://launch.yahoo.com >>>>> >>>>> >>> > _______________________________________________________________ >>>>> >>>>> Don't miss the 2002 Sprint PCS Application >>> Developer's Conference >>>>> August 25-28 in Las Vegas -- >>> http://devcon.sprintpcs.com/adp/index.cfm >>>>> >>>>> _______________________________________________ >>>>> Webwork-devel mailing list >>>>> Web...@li... >>>>> >>> > https://lists.sourceforge.net/lists/listinfo/webwork-devel >>>> >>>> >>>> >>> > _______________________________________________________________ >>>> >>>> Don't miss the 2002 Sprint PCS Application >>> Developer's Conference >>>> August 25-28 in Las Vegas -- >>> http://devcon.sprintpcs.com/adp/index.cfm >>>> >>>> _______________________________________________ >>>> Webwork-devel mailing list >>>> Web...@li... >>>> >>> > https://lists.sourceforge.net/lists/listinfo/webwork-devel >>> >>> >>> >>> > _______________________________________________________________ >>> >>> Don't miss the 2002 Sprint PCS Application > Developer's Conference >>> August 25-28 in Las Vegas -- > http://devcon.sprintpcs.com/adp/index.cfm >>> >>> _______________________________________________ >>> Webwork-devel mailing list >>> Web...@li... >>> > https://lists.sourceforge.net/lists/listinfo/webwork-devel > > > > _______________________________________________________________ > > Don't miss the 2002 Sprint PCS Application Developer's Conference > August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm > > _______________________________________________ > Webwork-devel mailing list > Web...@li... > https://lists.sourceforge.net/lists/listinfo/webwork-devel |
|
From: Matt B. <ma...@sm...> - 2002-05-27 11:38:13
|
I agree that reset logic should be moved to doEndTag() but this is different from null'ing attributes. Null'ing attributes should still be in release(). I don't want to make the blanket change at this time. I did fix Property and Action. I would prefer to make a couple of fixes and all containers should work. I changed the escaping of outer quotes to true by default. The reason it was false was just a first guess. I guessed wrong :). I haven't tested these changes on Resin yet. I can't get to caucho but I will by early this week. -Matt ----- Original Message ----- From: "Mike Cannon-Brookes" <mi...@at...> To: <ma...@sm...> Cc: <web...@li...> Sent: Monday, May 27, 2002 2:50 AM Subject: Re: [Webwork-devel] Webwork 1.0.3 / CVS taglib totally broken? > Ok - I've checked and from preliminary looks it seems like the ComponentTag > is fixed in Orion (haven't tested Resin yet - will do) > > PropertyTag: > I had to fix the PropertyTag again though. > > Simple fix: (can't check in myself but here's the details) > - remove return statement that's not at the end of the doEndTag method) > - call release() manually before return EVAL_PAGE; > (see attachment) > > Again - the problem here is that data is reset in the release() method which > is incorrect ;) > > I have a suggested fix for every tag - rename ALL the release() methods to > reset() and call them manually at the end of the doEndTag() methods (or > before every return EVAL_PAGE statement) as well. This keeps the 'reset' > logic compartmentalised but at the same time makes sure that it will be > called on ALL containers. > > I've done this in the attached file so you can see what I mean. > > TextUtil: > Also I changed the default value of "escapeOuterQuotes" to true. Why would > you ever want this to be false? (Having it false by default means for > example that any formfield with "text" as a value only gets text which is v. > dangerous) Can we change this in CVS? Why was escapeOuterQuotes ever needed? > > > I'll yell again if I find any more specific problems - but I can see that > the release() method does things in other tag (which I probably don't use) > which is bad. > > Cheers, > Mike > > PS More investigating (this time in Resin) since I wrote the above - there > are now a lot of NPEs due to the new ValueStack stuff. Line 96 of > ValueStack.java and line 403 of AbstractValueStack.java (both probably > related to null values being looked up and not checked for before calling > method on the value object) > > > Mike Cannon-Brookes > mi...@at... > > ATLASSIAN - Your J2EE Expert Partner > -------------------------------------------------------- > > Brilliant Software - http://www.atlassian.com/software > > Legendary Services - http://www.atlassian.com/support > > > On 24/5/02 1:03 AM, "ma...@sm..." (ma...@sm...) penned the > words: > > > Alright. Thanks for the reply. We'll get this worked > > out and commented accordingly. Whatever the outcome, > > I'll check it against Resin, WL, and Tomcat. I'll leave > > you and others to check it against Orion. As soon as I > > get a thumbs up from the community, I'll release a > > patch 1.0.4. > > > > -Matt > > > > On Fri, 24 May 2002, Mike Cannon-Brookes wrote > > > >> > >> Matt, > >> > >> Thanks mate - I'll check the CVS in the morning and > > report back. I found a > >> bunch of changes I needed to make today to get it > > working, so I'll see how > >> CVS works for me and start from there again tomorrow > > (with more mental > >> energy). > >> > >> Re: release() - this is NOT the preferred method at > > all ;) > >> > >> Again (yes, I've checked now with the spec and the > > author of JSP Tags book) > >> release() is called by the container to release > > resources associated with > >> the tag. It can be called at ANY time the container > > wishes (you have no > >> guarantee of when it is called). The only restriction > > is that it must be > >> called before the tag instance is garbage collected. > >> > >> So - what you have is lazy programmers (WL, Tomcat > > are the two glaring > >> examples) who just call release() after every tag > > invocation - which leads > >> to people's misplaced belief that it MUST be called > > everytime. This is not > >> so! :) Fast containers (like Resin and Orion) pool > > tag instances and reuse > >> them (maximising speed), and call release() as few > > times as possible > >> (sometimes never). I'm sure it's harder to code it > > this way, but the end > >> result is speed. > >> > >> Net result: > >> > >> NEVER do anything you want to happen in release()! > >> > >> This is true across all the JSP tags in WW and other > > projects. If you want > >> logic to occur, or variables to be reset for each tag > > invocation, reset them > >> at the end of the doEndTag() method (either before > > returning, or as a > >> finally block). > >> > >> I'll check CVS in the morning - good work on the > > quick fixes though! > >> > >> Cheers, > >> Mike > >> > >> On 23/5/02 10:20 PM, "ma...@sm..." > > (ma...@sm...) penned the > >> words: > >> > >>> For WebLogic, using release is the preferred > > method. I > >>> disagree with your blanket statement. I fixed the > >>> release order in the latest CVS and in the branch. > > I'm > >>> told that it is working correctly on Orion but I'm > >>> waiting for official confirmation. I would > > appreciate > >>> if you could check it with the latest CVS and let me > >>> know. > >>> > >>> On Thu, 23 May 2002, Mike Cannon-Brookes wrote > >>> > >>>> > >>>> Yes - release() is the bane of tag developers. > >>>> > >>>> The simple rule is basically - if you're using > >>> release, you're doing things > >>>> wrong! > >>>> > >>>> -mike > >>>> > >>>> On 23/5/02 4:28 PM, "Heng Sin Low" > >>> (low...@ya...) penned the words: > >>>> > >>>>> Ha, finally u got what I mean :) > >>>>> > >>>>> Yes, doEndTag should be the right place. I > > remember > >>>>> Matt have done a massive check in recently that > > move > >>>>> all this resetting stuff to the release() method. > >>>>> > >>>>> Regards, > >>>>> Low > >>>>> > >>>>>> I'm trying to what out what changes have occurred > >>>>> recently, but the taglib > >>>>> in CVS is currently broken and not backward > >>>>> compatible. > >>>>> > >>>>> > >>>>> __________________________________________________ > >>>>> Do You Yahoo!? > >>>>> LAUNCH - Your Yahoo! Music Experience > >>>>> http://launch.yahoo.com > >>>>> > >>>>> > >>> > > _______________________________________________________________ > >>>>> > >>>>> Don't miss the 2002 Sprint PCS Application > >>> Developer's Conference > >>>>> August 25-28 in Las Vegas -- > >>> http://devcon.sprintpcs.com/adp/index.cfm > >>>>> > >>>>> _______________________________________________ > >>>>> Webwork-devel mailing list > >>>>> Web...@li... > >>>>> > >>> > > https://lists.sourceforge.net/lists/listinfo/webwork-devel > >>>> > >>>> > >>>> > >>> > > _______________________________________________________________ > >>>> > >>>> Don't miss the 2002 Sprint PCS Application > >>> Developer's Conference > >>>> August 25-28 in Las Vegas -- > >>> http://devcon.sprintpcs.com/adp/index.cfm > >>>> > >>>> _______________________________________________ > >>>> Webwork-devel mailing list > >>>> Web...@li... > >>>> > >>> > > https://lists.sourceforge.net/lists/listinfo/webwork-devel > >>> > >>> > >>> > >>> > > _______________________________________________________________ > >>> > >>> Don't miss the 2002 Sprint PCS Application > > Developer's Conference > >>> August 25-28 in Las Vegas -- > > http://devcon.sprintpcs.com/adp/index.cfm > >>> > >>> _______________________________________________ > >>> Webwork-devel mailing list > >>> Web...@li... > >>> > > https://lists.sourceforge.net/lists/listinfo/webwork-devel > > > > > > > > _______________________________________________________________ > > > > Don't miss the 2002 Sprint PCS Application Developer's Conference > > August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm > > > > _______________________________________________ > > Webwork-devel mailing list > > Web...@li... > > https://lists.sourceforge.net/lists/listinfo/webwork-devel > > |
|
From: Mike Cannon-B. <mi...@at...> - 2002-05-28 00:26:06
|
On 27/5/02 9:38 PM, "Matt Baldree" (ma...@sm...) penned the words: > I agree that reset logic should be moved to doEndTag() but this is different > from null'ing attributes. Null'ing attributes should still be in release(). > I don't want to make the blanket change at this time. I did fix Property and > Action. I would prefer to make a couple of fixes and all containers should > work. I don't understand. Every webwork tag (that I can see) assumes that attributes are null at the start of doStartTag(). Moving everything into reset() and calling from doEndTag() and release() fixes this problem for all containers! Why would you not want to make this change? :) > I changed the escaping of outer quotes to true by default. The reason > it was false was just a first guess. I guessed wrong :). I haven't tested > these changes on Resin yet. I can't get to caucho but I will by early this > week. Cool - I'll update my CVS and try again today - another day battling Resin. ;) Cheers, Mike > -Matt > > ----- Original Message ----- > From: "Mike Cannon-Brookes" <mi...@at...> > To: <ma...@sm...> > Cc: <web...@li...> > Sent: Monday, May 27, 2002 2:50 AM > Subject: Re: [Webwork-devel] Webwork 1.0.3 / CVS taglib totally broken? > > >> Ok - I've checked and from preliminary looks it seems like the > ComponentTag >> is fixed in Orion (haven't tested Resin yet - will do) >> >> PropertyTag: >> I had to fix the PropertyTag again though. >> >> Simple fix: (can't check in myself but here's the details) >> - remove return statement that's not at the end of the doEndTag method) >> - call release() manually before return EVAL_PAGE; >> (see attachment) >> >> Again - the problem here is that data is reset in the release() method > which >> is incorrect ;) >> >> I have a suggested fix for every tag - rename ALL the release() methods to >> reset() and call them manually at the end of the doEndTag() methods (or >> before every return EVAL_PAGE statement) as well. This keeps the 'reset' >> logic compartmentalised but at the same time makes sure that it will be >> called on ALL containers. >> >> I've done this in the attached file so you can see what I mean. >> >> TextUtil: >> Also I changed the default value of "escapeOuterQuotes" to true. Why would >> you ever want this to be false? (Having it false by default means for >> example that any formfield with "text" as a value only gets text which is > v. >> dangerous) Can we change this in CVS? Why was escapeOuterQuotes ever > needed? >> >> >> I'll yell again if I find any more specific problems - but I can see that >> the release() method does things in other tag (which I probably don't use) >> which is bad. >> >> Cheers, >> Mike >> >> PS More investigating (this time in Resin) since I wrote the above - there >> are now a lot of NPEs due to the new ValueStack stuff. Line 96 of >> ValueStack.java and line 403 of AbstractValueStack.java (both probably >> related to null values being looked up and not checked for before calling >> method on the value object) >> >> >> Mike Cannon-Brookes >> mi...@at... >> >> ATLASSIAN - Your J2EE Expert Partner >> -------------------------------------------------------- >>> Brilliant Software - http://www.atlassian.com/software >>> Legendary Services - http://www.atlassian.com/support >> >> >> On 24/5/02 1:03 AM, "ma...@sm..." (ma...@sm...) penned the >> words: >> >>> Alright. Thanks for the reply. We'll get this worked >>> out and commented accordingly. Whatever the outcome, >>> I'll check it against Resin, WL, and Tomcat. I'll leave >>> you and others to check it against Orion. As soon as I >>> get a thumbs up from the community, I'll release a >>> patch 1.0.4. >>> >>> -Matt >>> >>> On Fri, 24 May 2002, Mike Cannon-Brookes wrote >>> >>>> >>>> Matt, >>>> >>>> Thanks mate - I'll check the CVS in the morning and >>> report back. I found a >>>> bunch of changes I needed to make today to get it >>> working, so I'll see how >>>> CVS works for me and start from there again tomorrow >>> (with more mental >>>> energy). >>>> >>>> Re: release() - this is NOT the preferred method at >>> all ;) >>>> >>>> Again (yes, I've checked now with the spec and the >>> author of JSP Tags book) >>>> release() is called by the container to release >>> resources associated with >>>> the tag. It can be called at ANY time the container >>> wishes (you have no >>>> guarantee of when it is called). The only restriction >>> is that it must be >>>> called before the tag instance is garbage collected. >>>> >>>> So - what you have is lazy programmers (WL, Tomcat >>> are the two glaring >>>> examples) who just call release() after every tag >>> invocation - which leads >>>> to people's misplaced belief that it MUST be called >>> everytime. This is not >>>> so! :) Fast containers (like Resin and Orion) pool >>> tag instances and reuse >>>> them (maximising speed), and call release() as few >>> times as possible >>>> (sometimes never). I'm sure it's harder to code it >>> this way, but the end >>>> result is speed. >>>> >>>> Net result: >>>> >>>> NEVER do anything you want to happen in release()! >>>> >>>> This is true across all the JSP tags in WW and other >>> projects. If you want >>>> logic to occur, or variables to be reset for each tag >>> invocation, reset them >>>> at the end of the doEndTag() method (either before >>> returning, or as a >>>> finally block). >>>> >>>> I'll check CVS in the morning - good work on the >>> quick fixes though! >>>> >>>> Cheers, >>>> Mike >>>> >>>> On 23/5/02 10:20 PM, "ma...@sm..." >>> (ma...@sm...) penned the >>>> words: >>>> >>>>> For WebLogic, using release is the preferred >>> method. I >>>>> disagree with your blanket statement. I fixed the >>>>> release order in the latest CVS and in the branch. >>> I'm >>>>> told that it is working correctly on Orion but I'm >>>>> waiting for official confirmation. I would >>> appreciate >>>>> if you could check it with the latest CVS and let me >>>>> know. >>>>> >>>>> On Thu, 23 May 2002, Mike Cannon-Brookes wrote >>>>> >>>>>> >>>>>> Yes - release() is the bane of tag developers. >>>>>> >>>>>> The simple rule is basically - if you're using >>>>> release, you're doing things >>>>>> wrong! >>>>>> >>>>>> -mike >>>>>> >>>>>> On 23/5/02 4:28 PM, "Heng Sin Low" >>>>> (low...@ya...) penned the words: >>>>>> >>>>>>> Ha, finally u got what I mean :) >>>>>>> >>>>>>> Yes, doEndTag should be the right place. I >>> remember >>>>>>> Matt have done a massive check in recently that >>> move >>>>>>> all this resetting stuff to the release() method. >>>>>>> >>>>>>> Regards, >>>>>>> Low >>>>>>> >>>>>>>> I'm trying to what out what changes have occurred >>>>>>> recently, but the taglib >>>>>>> in CVS is currently broken and not backward >>>>>>> compatible. >>>>>>> >>>>>>> >>>>>>> __________________________________________________ >>>>>>> Do You Yahoo!? >>>>>>> LAUNCH - Your Yahoo! Music Experience >>>>>>> http://launch.yahoo.com >>>>>>> >>>>>>> >>>>> >>> _______________________________________________________________ >>>>>>> >>>>>>> Don't miss the 2002 Sprint PCS Application >>>>> Developer's Conference >>>>>>> August 25-28 in Las Vegas -- >>>>> http://devcon.sprintpcs.com/adp/index.cfm >>>>>>> >>>>>>> _______________________________________________ >>>>>>> Webwork-devel mailing list >>>>>>> Web...@li... >>>>>>> >>>>> >>> https://lists.sourceforge.net/lists/listinfo/webwork-devel >>>>>> >>>>>> >>>>>> >>>>> >>> _______________________________________________________________ >>>>>> >>>>>> Don't miss the 2002 Sprint PCS Application >>>>> Developer's Conference >>>>>> August 25-28 in Las Vegas -- >>>>> http://devcon.sprintpcs.com/adp/index.cfm >>>>>> >>>>>> _______________________________________________ >>>>>> Webwork-devel mailing list >>>>>> Web...@li... >>>>>> >>>>> >>> https://lists.sourceforge.net/lists/listinfo/webwork-devel >>>>> >>>>> >>>>> >>>>> >>> _______________________________________________________________ >>>>> >>>>> Don't miss the 2002 Sprint PCS Application >>> Developer's Conference >>>>> August 25-28 in Las Vegas -- >>> http://devcon.sprintpcs.com/adp/index.cfm >>>>> >>>>> _______________________________________________ >>>>> Webwork-devel mailing list >>>>> Web...@li... >>>>> >>> https://lists.sourceforge.net/lists/listinfo/webwork-devel >>> >>> >>> >>> _______________________________________________________________ >>> >>> Don't miss the 2002 Sprint PCS Application Developer's Conference >>> August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm >>> >>> _______________________________________________ >>> Webwork-devel mailing list >>> Web...@li... >>> https://lists.sourceforge.net/lists/listinfo/webwork-devel >> >> > > > > _______________________________________________________________ > > Don't miss the 2002 Sprint PCS Application Developer's Conference > August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm > > _______________________________________________ > Webwork-devel mailing list > Web...@li... > https://lists.sourceforge.net/lists/listinfo/webwork-devel |
|
From: Matt B. <ma...@sm...> - 2002-05-28 00:59:11
|
Give me a few days to look at this. I'm not oppose to it. I just need time to test it all. -Matt ----- Original Message ----- From: "Mike Cannon-Brookes" <mi...@at...> To: <web...@li...> Sent: Monday, May 27, 2002 7:26 PM Subject: Re: [Webwork-devel] Webwork 1.0.3 / CVS taglib totally broken? > On 27/5/02 9:38 PM, "Matt Baldree" (ma...@sm...) penned the words: > > > I agree that reset logic should be moved to doEndTag() but this is different > > from null'ing attributes. Null'ing attributes should still be in release(). > > I don't want to make the blanket change at this time. I did fix Property and > > Action. I would prefer to make a couple of fixes and all containers should > > work. > > I don't understand. Every webwork tag (that I can see) assumes that > attributes are null at the start of doStartTag(). Moving everything into > reset() and calling from doEndTag() and release() fixes this problem for all > containers! > > Why would you not want to make this change? :) > > > I changed the escaping of outer quotes to true by default. The reason > > it was false was just a first guess. I guessed wrong :). I haven't tested > > these changes on Resin yet. I can't get to caucho but I will by early this > > week. > > Cool - I'll update my CVS and try again today - another day battling Resin. > ;) > > Cheers, > Mike > > > -Matt > > > > ----- Original Message ----- > > From: "Mike Cannon-Brookes" <mi...@at...> > > To: <ma...@sm...> > > Cc: <web...@li...> > > Sent: Monday, May 27, 2002 2:50 AM > > Subject: Re: [Webwork-devel] Webwork 1.0.3 / CVS taglib totally broken? > > > > > >> Ok - I've checked and from preliminary looks it seems like the > > ComponentTag > >> is fixed in Orion (haven't tested Resin yet - will do) > >> > >> PropertyTag: > >> I had to fix the PropertyTag again though. > >> > >> Simple fix: (can't check in myself but here's the details) > >> - remove return statement that's not at the end of the doEndTag method) > >> - call release() manually before return EVAL_PAGE; > >> (see attachment) > >> > >> Again - the problem here is that data is reset in the release() method > > which > >> is incorrect ;) > >> > >> I have a suggested fix for every tag - rename ALL the release() methods to > >> reset() and call them manually at the end of the doEndTag() methods (or > >> before every return EVAL_PAGE statement) as well. This keeps the 'reset' > >> logic compartmentalised but at the same time makes sure that it will be > >> called on ALL containers. > >> > >> I've done this in the attached file so you can see what I mean. > >> > >> TextUtil: > >> Also I changed the default value of "escapeOuterQuotes" to true. Why would > >> you ever want this to be false? (Having it false by default means for > >> example that any formfield with "text" as a value only gets text which is > > v. > >> dangerous) Can we change this in CVS? Why was escapeOuterQuotes ever > > needed? > >> > >> > >> I'll yell again if I find any more specific problems - but I can see that > >> the release() method does things in other tag (which I probably don't use) > >> which is bad. > >> > >> Cheers, > >> Mike > >> > >> PS More investigating (this time in Resin) since I wrote the above - there > >> are now a lot of NPEs due to the new ValueStack stuff. Line 96 of > >> ValueStack.java and line 403 of AbstractValueStack.java (both probably > >> related to null values being looked up and not checked for before calling > >> method on the value object) > >> > >> > >> Mike Cannon-Brookes > >> mi...@at... > >> > >> ATLASSIAN - Your J2EE Expert Partner > >> -------------------------------------------------------- > >>> Brilliant Software - http://www.atlassian.com/software > >>> Legendary Services - http://www.atlassian.com/support > >> > >> > >> On 24/5/02 1:03 AM, "ma...@sm..." (ma...@sm...) penned the > >> words: > >> > >>> Alright. Thanks for the reply. We'll get this worked > >>> out and commented accordingly. Whatever the outcome, > >>> I'll check it against Resin, WL, and Tomcat. I'll leave > >>> you and others to check it against Orion. As soon as I > >>> get a thumbs up from the community, I'll release a > >>> patch 1.0.4. > >>> > >>> -Matt > >>> > >>> On Fri, 24 May 2002, Mike Cannon-Brookes wrote > >>> > >>>> > >>>> Matt, > >>>> > >>>> Thanks mate - I'll check the CVS in the morning and > >>> report back. I found a > >>>> bunch of changes I needed to make today to get it > >>> working, so I'll see how > >>>> CVS works for me and start from there again tomorrow > >>> (with more mental > >>>> energy). > >>>> > >>>> Re: release() - this is NOT the preferred method at > >>> all ;) > >>>> > >>>> Again (yes, I've checked now with the spec and the > >>> author of JSP Tags book) > >>>> release() is called by the container to release > >>> resources associated with > >>>> the tag. It can be called at ANY time the container > >>> wishes (you have no > >>>> guarantee of when it is called). The only restriction > >>> is that it must be > >>>> called before the tag instance is garbage collected. > >>>> > >>>> So - what you have is lazy programmers (WL, Tomcat > >>> are the two glaring > >>>> examples) who just call release() after every tag > >>> invocation - which leads > >>>> to people's misplaced belief that it MUST be called > >>> everytime. This is not > >>>> so! :) Fast containers (like Resin and Orion) pool > >>> tag instances and reuse > >>>> them (maximising speed), and call release() as few > >>> times as possible > >>>> (sometimes never). I'm sure it's harder to code it > >>> this way, but the end > >>>> result is speed. > >>>> > >>>> Net result: > >>>> > >>>> NEVER do anything you want to happen in release()! > >>>> > >>>> This is true across all the JSP tags in WW and other > >>> projects. If you want > >>>> logic to occur, or variables to be reset for each tag > >>> invocation, reset them > >>>> at the end of the doEndTag() method (either before > >>> returning, or as a > >>>> finally block). > >>>> > >>>> I'll check CVS in the morning - good work on the > >>> quick fixes though! > >>>> > >>>> Cheers, > >>>> Mike > >>>> > >>>> On 23/5/02 10:20 PM, "ma...@sm..." > >>> (ma...@sm...) penned the > >>>> words: > >>>> > >>>>> For WebLogic, using release is the preferred > >>> method. I > >>>>> disagree with your blanket statement. I fixed the > >>>>> release order in the latest CVS and in the branch. > >>> I'm > >>>>> told that it is working correctly on Orion but I'm > >>>>> waiting for official confirmation. I would > >>> appreciate > >>>>> if you could check it with the latest CVS and let me > >>>>> know. > >>>>> > >>>>> On Thu, 23 May 2002, Mike Cannon-Brookes wrote > >>>>> > >>>>>> > >>>>>> Yes - release() is the bane of tag developers. > >>>>>> > >>>>>> The simple rule is basically - if you're using > >>>>> release, you're doing things > >>>>>> wrong! > >>>>>> > >>>>>> -mike > >>>>>> > >>>>>> On 23/5/02 4:28 PM, "Heng Sin Low" > >>>>> (low...@ya...) penned the words: > >>>>>> > >>>>>>> Ha, finally u got what I mean :) > >>>>>>> > >>>>>>> Yes, doEndTag should be the right place. I > >>> remember > >>>>>>> Matt have done a massive check in recently that > >>> move > >>>>>>> all this resetting stuff to the release() method. > >>>>>>> > >>>>>>> Regards, > >>>>>>> Low > >>>>>>> > >>>>>>>> I'm trying to what out what changes have occurred > >>>>>>> recently, but the taglib > >>>>>>> in CVS is currently broken and not backward > >>>>>>> compatible. > >>>>>>> > >>>>>>> > >>>>>>> __________________________________________________ > >>>>>>> Do You Yahoo!? > >>>>>>> LAUNCH - Your Yahoo! Music Experience > >>>>>>> http://launch.yahoo.com > >>>>>>> > >>>>>>> > >>>>> > >>> _______________________________________________________________ > >>>>>>> > >>>>>>> Don't miss the 2002 Sprint PCS Application > >>>>> Developer's Conference > >>>>>>> August 25-28 in Las Vegas -- > >>>>> http://devcon.sprintpcs.com/adp/index.cfm > >>>>>>> > >>>>>>> _______________________________________________ > >>>>>>> Webwork-devel mailing list > >>>>>>> Web...@li... > >>>>>>> > >>>>> > >>> https://lists.sourceforge.net/lists/listinfo/webwork-devel > >>>>>> > >>>>>> > >>>>>> > >>>>> > >>> _______________________________________________________________ > >>>>>> > >>>>>> Don't miss the 2002 Sprint PCS Application > >>>>> Developer's Conference > >>>>>> August 25-28 in Las Vegas -- > >>>>> http://devcon.sprintpcs.com/adp/index.cfm > >>>>>> > >>>>>> _______________________________________________ > >>>>>> Webwork-devel mailing list > >>>>>> Web...@li... > >>>>>> > >>>>> > >>> https://lists.sourceforge.net/lists/listinfo/webwork-devel > >>>>> > >>>>> > >>>>> > >>>>> > >>> _______________________________________________________________ > >>>>> > >>>>> Don't miss the 2002 Sprint PCS Application > >>> Developer's Conference > >>>>> August 25-28 in Las Vegas -- > >>> http://devcon.sprintpcs.com/adp/index.cfm > >>>>> > >>>>> _______________________________________________ > >>>>> Webwork-devel mailing list > >>>>> Web...@li... > >>>>> > >>> https://lists.sourceforge.net/lists/listinfo/webwork-devel > >>> > >>> > >>> > >>> _______________________________________________________________ > >>> > >>> Don't miss the 2002 Sprint PCS Application Developer's Conference > >>> August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm > >>> > >>> _______________________________________________ > >>> Webwork-devel mailing list > >>> Web...@li... > >>> https://lists.sourceforge.net/lists/listinfo/webwork-devel > >> > >> > > > > > > > > _______________________________________________________________ > > > > Don't miss the 2002 Sprint PCS Application Developer's Conference > > August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm > > > > _______________________________________________ > > Webwork-devel mailing list > > Web...@li... > > https://lists.sourceforge.net/lists/listinfo/webwork-devel > > > _______________________________________________________________ > > Don't miss the 2002 Sprint PCS Application Developer's Conference > August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm > > _______________________________________________ > Webwork-devel mailing list > Web...@li... > https://lists.sourceforge.net/lists/listinfo/webwork-devel > |
|
From: Mike Cannon-B. <mi...@at...> - 2002-05-28 00:44:32
|
I just looked at this again and the PropertyTag is _still_ broken I think.
Somehow I don't seem to be getting through here - my fix worked perfectly on
all containers, why was it not used?! :)
You still have valueAttr being set to null in the release() method.
However, property tag can be used like so <webwork:property /> but in this
case (as the valueAttr still has the value from the last time property was
used) the wrong value is used!
<ww:property value="foo">
<ww:property value="../bar" />
<ww:property /> <-- this should print use foo as the value correct? But
here it will use the 'cached' ../bar!
</ww:property>
Once again - everything should be reset in doEndTag! I cannot find a single
tag (browsing the ww source) that should EVER use release()!
-mike
On 27/5/02 9:38 PM, "Matt Baldree" (ma...@sm...) penned the words:
> I agree that reset logic should be moved to doEndTag() but this is different
> from null'ing attributes. Null'ing attributes should still be in release().
> I don't want to make the blanket change at this time. I did fix Property and
> Action. I would prefer to make a couple of fixes and all containers should
> work. I changed the escaping of outer quotes to true by default. The reason
> it was false was just a first guess. I guessed wrong :). I haven't tested
> these changes on Resin yet. I can't get to caucho but I will by early this
> week.
>
> -Matt
>
> ----- Original Message -----
> From: "Mike Cannon-Brookes" <mi...@at...>
> To: <ma...@sm...>
> Cc: <web...@li...>
> Sent: Monday, May 27, 2002 2:50 AM
> Subject: Re: [Webwork-devel] Webwork 1.0.3 / CVS taglib totally broken?
>
>
>> Ok - I've checked and from preliminary looks it seems like the
> ComponentTag
>> is fixed in Orion (haven't tested Resin yet - will do)
>>
>> PropertyTag:
>> I had to fix the PropertyTag again though.
>>
>> Simple fix: (can't check in myself but here's the details)
>> - remove return statement that's not at the end of the doEndTag method)
>> - call release() manually before return EVAL_PAGE;
>> (see attachment)
>>
>> Again - the problem here is that data is reset in the release() method
> which
>> is incorrect ;)
>>
>> I have a suggested fix for every tag - rename ALL the release() methods to
>> reset() and call them manually at the end of the doEndTag() methods (or
>> before every return EVAL_PAGE statement) as well. This keeps the 'reset'
>> logic compartmentalised but at the same time makes sure that it will be
>> called on ALL containers.
>>
>> I've done this in the attached file so you can see what I mean.
>>
>> TextUtil:
>> Also I changed the default value of "escapeOuterQuotes" to true. Why would
>> you ever want this to be false? (Having it false by default means for
>> example that any formfield with "text" as a value only gets text which is
> v.
>> dangerous) Can we change this in CVS? Why was escapeOuterQuotes ever
> needed?
>>
>>
>> I'll yell again if I find any more specific problems - but I can see that
>> the release() method does things in other tag (which I probably don't use)
>> which is bad.
>>
>> Cheers,
>> Mike
>>
>> PS More investigating (this time in Resin) since I wrote the above - there
>> are now a lot of NPEs due to the new ValueStack stuff. Line 96 of
>> ValueStack.java and line 403 of AbstractValueStack.java (both probably
>> related to null values being looked up and not checked for before calling
>> method on the value object)
>>
>>
>> Mike Cannon-Brookes
>> mi...@at...
>>
>> ATLASSIAN - Your J2EE Expert Partner
>> --------------------------------------------------------
>>> Brilliant Software - http://www.atlassian.com/software
>>> Legendary Services - http://www.atlassian.com/support
>>
>>
>> On 24/5/02 1:03 AM, "ma...@sm..." (ma...@sm...) penned the
>> words:
>>
>>> Alright. Thanks for the reply. We'll get this worked
>>> out and commented accordingly. Whatever the outcome,
>>> I'll check it against Resin, WL, and Tomcat. I'll leave
>>> you and others to check it against Orion. As soon as I
>>> get a thumbs up from the community, I'll release a
>>> patch 1.0.4.
>>>
>>> -Matt
>>>
>>> On Fri, 24 May 2002, Mike Cannon-Brookes wrote
>>>
>>>>
>>>> Matt,
>>>>
>>>> Thanks mate - I'll check the CVS in the morning and
>>> report back. I found a
>>>> bunch of changes I needed to make today to get it
>>> working, so I'll see how
>>>> CVS works for me and start from there again tomorrow
>>> (with more mental
>>>> energy).
>>>>
>>>> Re: release() - this is NOT the preferred method at
>>> all ;)
>>>>
>>>> Again (yes, I've checked now with the spec and the
>>> author of JSP Tags book)
>>>> release() is called by the container to release
>>> resources associated with
>>>> the tag. It can be called at ANY time the container
>>> wishes (you have no
>>>> guarantee of when it is called). The only restriction
>>> is that it must be
>>>> called before the tag instance is garbage collected.
>>>>
>>>> So - what you have is lazy programmers (WL, Tomcat
>>> are the two glaring
>>>> examples) who just call release() after every tag
>>> invocation - which leads
>>>> to people's misplaced belief that it MUST be called
>>> everytime. This is not
>>>> so! :) Fast containers (like Resin and Orion) pool
>>> tag instances and reuse
>>>> them (maximising speed), and call release() as few
>>> times as possible
>>>> (sometimes never). I'm sure it's harder to code it
>>> this way, but the end
>>>> result is speed.
>>>>
>>>> Net result:
>>>>
>>>> NEVER do anything you want to happen in release()!
>>>>
>>>> This is true across all the JSP tags in WW and other
>>> projects. If you want
>>>> logic to occur, or variables to be reset for each tag
>>> invocation, reset them
>>>> at the end of the doEndTag() method (either before
>>> returning, or as a
>>>> finally block).
>>>>
>>>> I'll check CVS in the morning - good work on the
>>> quick fixes though!
>>>>
>>>> Cheers,
>>>> Mike
>>>>
>>>> On 23/5/02 10:20 PM, "ma...@sm..."
>>> (ma...@sm...) penned the
>>>> words:
>>>>
>>>>> For WebLogic, using release is the preferred
>>> method. I
>>>>> disagree with your blanket statement. I fixed the
>>>>> release order in the latest CVS and in the branch.
>>> I'm
>>>>> told that it is working correctly on Orion but I'm
>>>>> waiting for official confirmation. I would
>>> appreciate
>>>>> if you could check it with the latest CVS and let me
>>>>> know.
>>>>>
>>>>> On Thu, 23 May 2002, Mike Cannon-Brookes wrote
>>>>>
>>>>>>
>>>>>> Yes - release() is the bane of tag developers.
>>>>>>
>>>>>> The simple rule is basically - if you're using
>>>>> release, you're doing things
>>>>>> wrong!
>>>>>>
>>>>>> -mike
>>>>>>
>>>>>> On 23/5/02 4:28 PM, "Heng Sin Low"
>>>>> (low...@ya...) penned the words:
>>>>>>
>>>>>>> Ha, finally u got what I mean :)
>>>>>>>
>>>>>>> Yes, doEndTag should be the right place. I
>>> remember
>>>>>>> Matt have done a massive check in recently that
>>> move
>>>>>>> all this resetting stuff to the release() method.
>>>>>>>
>>>>>>> Regards,
>>>>>>> Low
>>>>>>>
>>>>>>>> I'm trying to what out what changes have occurred
>>>>>>> recently, but the taglib
>>>>>>> in CVS is currently broken and not backward
>>>>>>> compatible.
>>>>>>>
>>>>>>>
>>>>>>> __________________________________________________
>>>>>>> Do You Yahoo!?
>>>>>>> LAUNCH - Your Yahoo! Music Experience
>>>>>>> http://launch.yahoo.com
>>>>>>>
>>>>>>>
>>>>>
>>> _______________________________________________________________
>>>>>>>
>>>>>>> Don't miss the 2002 Sprint PCS Application
>>>>> Developer's Conference
>>>>>>> August 25-28 in Las Vegas --
>>>>> http://devcon.sprintpcs.com/adp/index.cfm
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> Webwork-devel mailing list
>>>>>>> Web...@li...
>>>>>>>
>>>>>
>>> https://lists.sourceforge.net/lists/listinfo/webwork-devel
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>> _______________________________________________________________
>>>>>>
>>>>>> Don't miss the 2002 Sprint PCS Application
>>>>> Developer's Conference
>>>>>> August 25-28 in Las Vegas --
>>>>> http://devcon.sprintpcs.com/adp/index.cfm
>>>>>>
>>>>>> _______________________________________________
>>>>>> Webwork-devel mailing list
>>>>>> Web...@li...
>>>>>>
>>>>>
>>> https://lists.sourceforge.net/lists/listinfo/webwork-devel
>>>>>
>>>>>
>>>>>
>>>>>
>>> _______________________________________________________________
>>>>>
>>>>> Don't miss the 2002 Sprint PCS Application
>>> Developer's Conference
>>>>> August 25-28 in Las Vegas --
>>> http://devcon.sprintpcs.com/adp/index.cfm
>>>>>
>>>>> _______________________________________________
>>>>> Webwork-devel mailing list
>>>>> Web...@li...
>>>>>
>>> https://lists.sourceforge.net/lists/listinfo/webwork-devel
>>>
>>>
>>>
>>> _______________________________________________________________
>>>
>>> Don't miss the 2002 Sprint PCS Application Developer's Conference
>>> August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm
>>>
>>> _______________________________________________
>>> Webwork-devel mailing list
>>> Web...@li...
>>> https://lists.sourceforge.net/lists/listinfo/webwork-devel
>>
>>
>
>
>
> _______________________________________________________________
>
> Don't miss the 2002 Sprint PCS Application Developer's Conference
> August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm
>
> _______________________________________________
> Webwork-devel mailing list
> Web...@li...
> https://lists.sourceforge.net/lists/listinfo/webwork-devel
|
|
From: Sven K. <sv...@im...> - 2002-05-27 17:13:38
|
"Mike Cannon-Brookes" <mi...@at...> schrieb im Newsbeitrag
news:B9182255.B7E7%mi...@at......
> PS More investigating (this time in Resin) since I wrote the above - there
> are now a lot of NPEs due to the new ValueStack stuff. Line 96 of
> ValueStack.java and line 403 of AbstractValueStack.java (both probably
> related to null values being looked up and not checked for before calling
> method on the value object)
Strange. Both lines you mention are unchanged to the old ValueStack Besides
the fact that some of them now live in an abstract class, that is. Are you
sure the NPE's aren't occurring with the old ValueStack as well?
Sven....
|
|
From: Mike Cannon-B. <mi...@at...> - 2002-05-23 14:50:15
|
Matt, Thanks mate - I'll check the CVS in the morning and report back. I found a bunch of changes I needed to make today to get it working, so I'll see how CVS works for me and start from there again tomorrow (with more mental energy). Re: release() - this is NOT the preferred method at all ;) Again (yes, I've checked now with the spec and the author of JSP Tags book) release() is called by the container to release resources associated with the tag. It can be called at ANY time the container wishes (you have no guarantee of when it is called). The only restriction is that it must be called before the tag instance is garbage collected. So - what you have is lazy programmers (WL, Tomcat are the two glaring examples) who just call release() after every tag invocation - which leads to people's misplaced belief that it MUST be called everytime. This is not so! :) Fast containers (like Resin and Orion) pool tag instances and reuse them (maximising speed), and call release() as few times as possible (sometimes never). I'm sure it's harder to code it this way, but the end result is speed. Net result: NEVER do anything you want to happen in release()! This is true across all the JSP tags in WW and other projects. If you want logic to occur, or variables to be reset for each tag invocation, reset them at the end of the doEndTag() method (either before returning, or as a finally block). I'll check CVS in the morning - good work on the quick fixes though! Cheers, Mike On 23/5/02 10:20 PM, "ma...@sm..." (ma...@sm...) penned the words: > For WebLogic, using release is the preferred method. I > disagree with your blanket statement. I fixed the > release order in the latest CVS and in the branch. I'm > told that it is working correctly on Orion but I'm > waiting for official confirmation. I would appreciate > if you could check it with the latest CVS and let me > know. > > On Thu, 23 May 2002, Mike Cannon-Brookes wrote > >> >> Yes - release() is the bane of tag developers. >> >> The simple rule is basically - if you're using > release, you're doing things >> wrong! >> >> -mike >> >> On 23/5/02 4:28 PM, "Heng Sin Low" > (low...@ya...) penned the words: >> >>> Ha, finally u got what I mean :) >>> >>> Yes, doEndTag should be the right place. I remember >>> Matt have done a massive check in recently that move >>> all this resetting stuff to the release() method. >>> >>> Regards, >>> Low >>> >>>> I'm trying to what out what changes have occurred >>> recently, but the taglib >>> in CVS is currently broken and not backward >>> compatible. >>> >>> >>> __________________________________________________ >>> Do You Yahoo!? >>> LAUNCH - Your Yahoo! Music Experience >>> http://launch.yahoo.com >>> >>> > _______________________________________________________________ >>> >>> Don't miss the 2002 Sprint PCS Application > Developer's Conference >>> August 25-28 in Las Vegas -- > http://devcon.sprintpcs.com/adp/index.cfm >>> >>> _______________________________________________ >>> Webwork-devel mailing list >>> Web...@li... >>> > https://lists.sourceforge.net/lists/listinfo/webwork-devel >> >> >> > _______________________________________________________________ >> >> Don't miss the 2002 Sprint PCS Application > Developer's Conference >> August 25-28 in Las Vegas -- > http://devcon.sprintpcs.com/adp/index.cfm >> >> _______________________________________________ >> Webwork-devel mailing list >> Web...@li... >> > https://lists.sourceforge.net/lists/listinfo/webwork-devel > > > > _______________________________________________________________ > > Don't miss the 2002 Sprint PCS Application Developer's Conference > August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm > > _______________________________________________ > Webwork-devel mailing list > Web...@li... > https://lists.sourceforge.net/lists/listinfo/webwork-devel |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X