AW: [vote] Release Wicket 1.4.9
[x] Yes, release Stefan
wicket examples on WicketStuff
Hello, the first three examples here http://www.wicketstuff.org/wicket14/repeater/ seem to be broken. At least when accessing it I get an Internal Error. Could someone with please have a look at this? Best, Korbinian
Re: wicket examples on WicketStuff
You can run them pretty easily yourself: svn co http://svn.apache.org/repos/asf/wicket/branches/wicket-1.4.x http://svn.apache.org/repos/asf/wicket/branches/wicket-1.4.xcd wicket-1.4.x mvn install cd wicket-examples mvn jetty:run Now, go to http://localhost:8080/wicket-examples -- Jeremy Thomerson http://www.wickettraining.com On Wed, May 19, 2010 at 2:39 AM, Korbinian Bachl - privat korbinian.ba...@whiskyworld.de wrote: Hello, the first three examples here http://www.wicketstuff.org/wicket14/repeater/ seem to be broken. At least when accessing it I get an Internal Error. Could someone with please have a look at this? Best, Korbinian
Re: wicket examples on WicketStuff
cool - one of those guys should notice. i just thought you might need to see the examples NOW :) -- Jeremy Thomerson http://www.wickettraining.com On Wed, May 19, 2010 at 3:03 AM, Korbinian Bachl - whiskyworld korbinian.ba...@whiskyworld.de wrote: Hi Jeremy, I know that I can do :) I came over it as I put a link to the examples in the ##wicket channel yesterday as someone asked about some kind of repeaters - and localhost isn't the best option then to show ;) Just wanted to let the person running it know, Best Jeremy Thomerson schrieb: You can run them pretty easily yourself: svn co http://svn.apache.org/repos/asf/wicket/branches/wicket-1.4.x http://svn.apache.org/repos/asf/wicket/branches/wicket-1.4.xcd wicket-1.4.x mvn install cd wicket-examples mvn jetty:run Now, go to http://localhost:8080/wicket-examples -- Jeremy Thomerson http://www.wickettraining.com On Wed, May 19, 2010 at 2:39 AM, Korbinian Bachl - privat korbinian.ba...@whiskyworld.de wrote: Hello, the first three examples here http://www.wicketstuff.org/wicket14/repeater/ seem to be broken. At least when accessing it I get an Internal Error. Could someone with please have a look at this? Best, Korbinian -- whiskyworld e.K. http://www.whiskyworld.de Ziegelfeld 6 94481 Grafenau/ Haus im Wald Tel. 08555/ 406 320 Fax. 08555/ 406 319 Amtsgericht Passau: HRA 11760 Geschäftsführer: Ulrike Bachl UstID: DE260619452
700+ page application switches from 1.3.7 to 1.4.8
Just two hours ago one of our developers pulled the trigger and committed his Wicket 1.4.8 porting work for our flagship application EduArte to our trunk. This means that as of this moment we finally are near trunk and can start uncovering and fixing bugs for 1.4.x. We expect to start work on porting our application towards 1.5 when we have stabilized our 1.4.8 version. A couple of statistics for our application: - ~725 pages - ~950 entities - hibernate - spring 3 - wicket security - wiquery The porting effort took a couple of months, tiered in 2 phases: convert our framework (about a month), convert our application (about a month). Now the application needs to prove itself in testing and will probably go live in a couple of weeks. Martijn
WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
Hi! I see that on the to be released 1.4.9 and worried. Was its consequences really thought about? I do think this can introduce memory leaks, when some Java code creates threads (like Java 2D code when dealing with images) that never dies. Or what am I missing? Adriano
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
I still don't see why using InheritableThreadLocal would introduce memory leaks when dealing with threads. Do you have a good example to prove it? I don't see any difference... -- View this message in context: http://apache-wicket.1842946.n4.nabble.com/WICKET-2846-Store-Application-in-InheritableThreadLocal-instead-of-ThreadLocal-tp639p771.html Sent from the Wicket - Dev mailing list archive at Nabble.com.
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
On 19/05/2010 09:50, Alex Objelean wrote: I still don't see why using InheritableThreadLocal would introduce memory leaks when dealing with threads. Do you have a good example to prove it? I don't see any difference... The application instance would go to arbitrary (like ones possible created by Java core library) and would never be garbaged collected. Adriano
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
Why would the application object itself need to be garbage collected? On Wed, May 19, 2010 at 8:53 AM, Adriano dos Santos Fernandes adrian...@gmail.com wrote: On 19/05/2010 09:50, Alex Objelean wrote: I still don't see why using InheritableThreadLocal would introduce memory leaks when dealing with threads. Do you have a good example to prove it? I don't see any difference... The application instance would go to arbitrary (like ones possible created by Java core library) and would never be garbaged collected. Adriano
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
On 19/05/2010 10:57, James Carman wrote: Why would the application object itself need to be garbage collected? To hot-deployment not eat your memory. Adriano PS: I'm much more worried in production environments. Restarting the container because you need to update the application is something I consider an awful practice.
Re: 700+ page application switches from 1.3.7 to 1.4.8
Sounds good, so do you expect that you'll find some bugs in core wicket that the community will benefit from? btw, just back from geecon where this speaker , Eugene Ciurana ( http://2010.geecon.org/speakerdetails/12 ) mentioned (very discretely) wicket in conjunction with jboss as his preferred stable stack. 2010/5/19 Martijn Dashorst martijn.dasho...@gmail.com: Just two hours ago one of our developers pulled the trigger and committed his Wicket 1.4.8 porting work for our flagship application EduArte to our trunk. This means that as of this moment we finally are near trunk and can start uncovering and fixing bugs for 1.4.x. We expect to start work on porting our application towards 1.5 when we have stabilized our 1.4.8 version. A couple of statistics for our application: - ~725 pages - ~950 entities - hibernate - spring 3 - wicket security - wiquery The porting effort took a couple of months, tiered in 2 phases: convert our framework (about a month), convert our application (about a month). Now the application needs to prove itself in testing and will probably go live in a couple of weeks. Martijn
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
Since you've never needed the Application instance in threads you had created, why would you need it from now on? The only problem related to memory leaks may be caused by your custom implementation, not with the fact of using InheritableThreadLocal. -- View this message in context: http://apache-wicket.1842946.n4.nabble.com/WICKET-2846-Store-Application-in-InheritableThreadLocal-instead-of-ThreadLocal-tp639p2223064.html Sent from the Wicket - Dev mailing list archive at Nabble.com.
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
On 19/05/2010 12:48, Alex Objelean wrote: Since you've never needed the Application instance in threads you had created, why would you need it from now on? The only problem related to memory leaks may be caused by your custom implementation, not with the fact of using InheritableThreadLocal. * This class extends ttThreadLocal/tt to provide inheritance of values * from parent thread to child thread: when a child thread is created, the * child receives initial values for all inheritable thread-local variables * for which the parent has values. Normally the child's values will be * identical to the parent's; however, the child's value can be made an * arbitrary function of the parent's by overriding the ttchildValue/tt * method in this class. As soon a child thread is created, the application will become a root of it and will never be collected, even if you not use it. Adriano
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
On 19/05/2010 12:50, James Carman wrote: Use JRebel! Problem solved. :) I suggest to not change something in a minor release that breaks things that is working. I also suggest this shouldn't be done by default in major releases as well. I see no way JRebel or any tool going to remove thread locals knowing what its being done. Adriano
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
Please, correct me if I'm wrong, but the Application won't become the root of child threads. Using InheritableThreadLocal will only make Application available from the threads created during a request cycle. There is absolutely no memory related problem with it. Alex -- View this message in context: http://apache-wicket.1842946.n4.nabble.com/WICKET-2846-Store-Application-in-InheritableThreadLocal-instead-of-ThreadLocal-tp639p2223101.html Sent from the Wicket - Dev mailing list archive at Nabble.com.
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
On 19/05/2010 13:06, Alex Objelean wrote: Please, correct me if I'm wrong, but the Application won't become the root of child threads. Using InheritableThreadLocal will only make Application available from the threads created during a request cycle. There is absolutely no memory related problem with it. As I said, some operations may spawn threads, like manipulation images, for example. If your application call it (during a request cycle), the application object will be stored in a system thread. Take a look at this: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6489540 This currently make web-classloader leaks. If you start using InheritableThreadLocal's with arbitrary objects, you're going to make even more leaks. Also note, there is something not good here. AFAIK, Wicket sets the thread locals only during the request. But if child threads are spawned, they can't be cleaned automatically. IMO, it should be done something that the user needs to call to set/clear this, like a specialized WicketThread class. Adriano
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
On Wed, May 19, 2010 at 11:56 AM, Adriano dos Santos Fernandes adrian...@gmail.com wrote: I suggest to not change something in a minor release that breaks things that is working. I also suggest this shouldn't be done by default in major releases as well. I see no way JRebel or any tool going to remove thread locals knowing what its being done. JRebel won't require totally blowing away your application object. It'll just swap out the underlying logic behind it when you restart your application.
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
On 19/05/2010 14:35, James Carman wrote: On Wed, May 19, 2010 at 11:56 AM, Adriano dos Santos Fernandes adrian...@gmail.com wrote: I suggest to not change something in a minor release that breaks things that is working. I also suggest this shouldn't be done by default in major releases as well. I see no way JRebel or any tool going to remove thread locals knowing what its being done. JRebel won't require totally blowing away your application object. It'll just swap out the underlying logic behind it when you restart your application. AFAIU, JRebel has nothing to do with the problem I referred, sorry. I've no problem with redeploy, at least, not caused by Wicket so far, and it would be very desirable that Wicket doesn't go this way. Adriano
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
JRebel is intended to be used in development, not in production. In production, you want to undeploy and redeploy your application and, hopefully, leave no old ClassLoader reference behind. I'm not sure if InheritableThreadLocal will create memory leaks, but I know it is something to be very carefully considered. On Wed, May 19, 2010 at 2:35 PM, James Carman ja...@carmanconsulting.comwrote: On Wed, May 19, 2010 at 11:56 AM, Adriano dos Santos Fernandes adrian...@gmail.com wrote: I suggest to not change something in a minor release that breaks things that is working. I also suggest this shouldn't be done by default in major releases as well. I see no way JRebel or any tool going to remove thread locals knowing what its being done. JRebel won't require totally blowing away your application object. It'll just swap out the underlying logic behind it when you restart your application.
Re: [vote] Release Wicket 1.4.9
How about we put the request back on you: Please prove that WICKET-2846 DOES introduce a nasty regression. This is much easier to prove / disprove than what you request. -- Jeremy Thomerson http://www.wickettraining.com On Wed, May 19, 2010 at 12:43 PM, Adriano dos Santos Fernandes adrian...@gmail.com wrote: On 19/05/2010 04:35, Jeremy Thomerson wrote: This vote is to release wicket 1.4.9 Branch: http://svn.apache.org/repos/asf/wicket/branches/wicket-1.4.9/ Artifacts: http://people.apache.org/~jrthomerson/releases/apache-wicket-1.4.9/dist Maven repo: http://people.apache.org/~jrthomerson/releases/apache-wicket-1.4.9/m2-repo Changelog: https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12310561styleName=Htmlversion=12314962 (and below) This vote ends Saturday 2:30am GMT-5 Please test the release and offer your vote: [ ] Yes, release [ ] No, don't release it (and please tell us why) [X ] No, don't release it, at least until someone guarantees WICKET-2846 is not going to introduce a nasty regression. Adriano
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
If I understand it correctly, the InheritedThreadLocal (ITL) wil hold the Application reference only if its corresponding thread stays alive. If the thread dies, the ITL value is eligible for gc, right? Well, if your application creates a new thread, that is kept alive even after the webapp is undeployed, the leak is the living thread, not the ITL. This thread will probably also make some reference to application classes (if not, why was it created in the first place?), which will cause the same memory leak. And, if the thread is taken from a pool managed by some lightweight container, e.g. Spring, the ITL will not inherit the Application value, since the 'working' thread is not a child of the request thread. Well, maybe there is some corner case, for example, if the thread pool implementation creates new threads on demand, and make them children of the requesting threads and thus, inheriting the Application instance (this should probably be considered a bug of the pool implementation). Even then, it will only be a problem if the pool is managed by something outside the application, because its threads must survive the web app undeployment to cause a leak. Well, I tried to think of some problematic case (since I was worried about this, too), but couldn't. Do you have any case with potential to trigger the leak? Tetsuo On Wed, May 19, 2010 at 1:15 PM, Adriano dos Santos Fernandes adrian...@gmail.com wrote: On 19/05/2010 13:06, Alex Objelean wrote: Please, correct me if I'm wrong, but the Application won't become the root of child threads. Using InheritableThreadLocal will only make Application available from the threads created during a request cycle. There is absolutely no memory related problem with it. As I said, some operations may spawn threads, like manipulation images, for example. If your application call it (during a request cycle), the application object will be stored in a system thread. Take a look at this: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6489540 This currently make web-classloader leaks. If you start using InheritableThreadLocal's with arbitrary objects, you're going to make even more leaks. Also note, there is something not good here. AFAIK, Wicket sets the thread locals only during the request. But if child threads are spawned, they can't be cleaned automatically. IMO, it should be done something that the user needs to call to set/clear this, like a specialized WicketThread class. Adriano
Re: [vote] Release Wicket 1.4.9
On 19/05/2010 14:49, Jeremy Thomerson wrote: How about we put the request back on you: Please prove that WICKET-2846 DOES introduce a nasty regression. This is much easier to prove / disprove than what you request. Jeremy, as an open source developer, I understand your position and what you said. However, I also understand that the Wicket team could consider the problem I'm bringing to you. I'm sure your team could understand what I said, and may explain if I'm wrong. My experience with this, however, is that it make Wicket another piece of the stack to introduce an evil problem. See the bug I shown, for example. Guice team just make they counterpart open till now. Wicket could do the same, but as this is a new improvement, it looks for me that its consequence was not well know, as this isn't essential for the framework. As a user, needing to do our applications, we haven't even time to upgrade, so we're still in 1.4.1. As my vote didn't really count (it's non binding, after all), voting against is just a way to show my in-satisfaction with something I considered a problem. Note that I have (re)opened bug reports that matter for us, and I'm not one that vote against due to it. Adriano
Re: [vote] Release Wicket 1.4.9
Do you have a quickstart application that exhibits this behavior? On Wed, May 19, 2010 at 2:44 PM, Adriano dos Santos Fernandes adrian...@gmail.com wrote: On 19/05/2010 14:49, Jeremy Thomerson wrote: How about we put the request back on you: Please prove that WICKET-2846 DOES introduce a nasty regression. This is much easier to prove / disprove than what you request. Jeremy, as an open source developer, I understand your position and what you said. However, I also understand that the Wicket team could consider the problem I'm bringing to you. I'm sure your team could understand what I said, and may explain if I'm wrong. My experience with this, however, is that it make Wicket another piece of the stack to introduce an evil problem. See the bug I shown, for example. Guice team just make they counterpart open till now. Wicket could do the same, but as this is a new improvement, it looks for me that its consequence was not well know, as this isn't essential for the framework. As a user, needing to do our applications, we haven't even time to upgrade, so we're still in 1.4.1. As my vote didn't really count (it's non binding, after all), voting against is just a way to show my in-satisfaction with something I considered a problem. Note that I have (re)opened bug reports that matter for us, and I'm not one that vote against due to it. Adriano
Re: [vote] Release Wicket 1.4.9
On 19/05/2010 15:48, James Carman wrote: Do you have a quickstart application that exhibits this behavior? How would I could do it? This requires someone willing to change the code (if there is a problem) to redeploy the application with have previously created a thread. About the sun bug, it is still open, so you may pass the ball or deal with it. Adriano
Re: [vote] Release Wicket 1.4.9
Here's how you create a quickstart: http://www.jeremythomerson.com/blog/2008/11/wicket-quickstart-tutorial/ If you find that there is a bug, you zip your quickstart directory and attach it to a JIRA issue. Then we fix it and build a new release and start a new vote (if the bug is serious enough). -- Jeremy Thomerson http://www.wickettraining.com On Wed, May 19, 2010 at 1:55 PM, Adriano dos Santos Fernandes adrian...@gmail.com wrote: On 19/05/2010 15:48, James Carman wrote: Do you have a quickstart application that exhibits this behavior? How would I could do it? This requires someone willing to change the code (if there is a problem) to redeploy the application with have previously created a thread. About the sun bug, it is still open, so you may pass the ball or deal with it. Adriano
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
- Original message - On 19/05/2010 13:06, Alex Objelean wrote: This currently make web-classloader leaks. If you start using InheritableThreadLocal's with arbitrary objects, you're going to make even more leaks. Also note, there is something not good here. AFAIK, Wicket sets the thread locals only during the request. But if child threads are spawned, they can't be cleaned automatically. IMO, it should be done something that the user needs to call to set/clear this, like a specialized WicketThread class. And when should that one clean up the threadlocal?? What would be a good time to clean it up? There would only be 1 place thats when then run method is finished. But if thats the case the thread and the threadlocals are cleared any way. if you would use a thread pool then you have to use otherways any way to se the Application threadlocal. Thread pools have call backs when a runnable is being executed and finished. But threads that are created in a request should finish and terminate at one point and never be reused.
Re: [vote] Release Wicket 1.4.9
On 19/05/2010 15:57, Jeremy Thomerson wrote: Here's how you create a quickstart: http://www.jeremythomerson.com/blog/2008/11/wicket-quickstart-tutorial/ If you find that there is a bug, you zip your quickstart directory and attach it to a JIRA issue. Then we fix it and build a new release and start a new vote (if the bug is serious enough). I know, I know... For example, the ones I have in Jira and got closed without analyze, or the ones I attached with fixes. I can't, however, do it now. But I would have no reason to do it knowing some folks just consider a normal thing restart the container to update the application. So if Wicket devs are in this way, I could write no quickstart to convince. Guice, by its use of thread locals, and considering that Java thread locals are not ideal, have the same type of bug. They could solve it with an API to close a thing, but they don't. They could ship some fancy classes that may work (accordingly to old @crazybob says) in some cases but they also didn't. So if you redeploy an app with Guice in Tomcat, it logs about a thread local leak. It's going to do the same thing with Wicket. Adriano
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
On 19/05/2010 16:16, Johan Compagner wrote: - Original message - On 19/05/2010 13:06, Alex Objelean wrote: This currently make web-classloader leaks. If you start using InheritableThreadLocal's with arbitrary objects, you're going to make even more leaks. Also note, there is something not good here. AFAIK, Wicket sets the thread locals only during the request. But if child threads are spawned, they can't be cleaned automatically. IMO, it should be done something that the user needs to call to set/clear this, like a specialized WicketThread class. And when should that one clean up the threadlocal?? What would be a good time to clean it up? That the user decides. Wicket could allow it to set/clear the TLS application. I do not think the application object is general enough to be passed implicitly to threads. BTW, can't a web application have more than one Wicket filters and then more than one application object? In this case, threads shared by the web application would have arbitrary application objects. But threads that are created in a request should finish and terminate at one point and never be reused. I already shown Java 1.4 bug. They seems not interested to change it, so you deal with it, you say Java is bad, or you're part of the restart is ok people. Adriano
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
What itch are we trying to scratch, here, anyway? When do folks need access to the Application object outside of a request thread? What is the usecase? On Wed, May 19, 2010 at 3:30 PM, Adriano dos Santos Fernandes adrian...@gmail.com wrote: On 19/05/2010 16:16, Johan Compagner wrote: - Original message - On 19/05/2010 13:06, Alex Objelean wrote: This currently make web-classloader leaks. If you start using InheritableThreadLocal's with arbitrary objects, you're going to make even more leaks. Also note, there is something not good here. AFAIK, Wicket sets the thread locals only during the request. But if child threads are spawned, they can't be cleaned automatically. IMO, it should be done something that the user needs to call to set/clear this, like a specialized WicketThread class. And when should that one clean up the threadlocal?? What would be a good time to clean it up? That the user decides. Wicket could allow it to set/clear the TLS application. I do not think the application object is general enough to be passed implicitly to threads. BTW, can't a web application have more than one Wicket filters and then more than one application object? In this case, threads shared by the web application would have arbitrary application objects. But threads that are created in a request should finish and terminate at one point and never be reused. I already shown Java 1.4 bug. They seems not interested to change it, so you deal with it, you say Java is bad, or you're part of the restart is ok people. Adriano
Re: [vote] Release Wicket 1.4.9
On Wed, May 19, 2010 at 4:20 PM, Adriano dos Santos Fernandes adrian...@gmail.com wrote: I can't, however, do it now. But I would have no reason to do it knowing some folks just consider a normal thing restart the container to update the application. So if Wicket devs are in this way, I could write no quickstart to convince. Cara, agora você está sendo um grande babacão... What they are trying to say is that there is no reason to believe that there is an issue regarding the InheritableThreadLocal and application redeploys, but your *unproven* doubt, thus submitting a test case would make the issue clear. Guice, by its use of thread locals, and considering that Java thread locals are not ideal, have the same type of bug. They could solve it with an API to close a thing, but they don't. They could ship some fancy classes that may work (accordingly to old @crazybob says) in some cases but they also didn't. So if you redeploy an app with Guice in Tomcat, it logs about a thread local leak. Is Guice's issue related to child thread creation (thus related to our issue)? Or is it just some clean-up they don't do (which is not related to our issue at all)? It's going to do the same thing with Wicket. Only if you leave request-created threads to run indefinitely. But this would be a bug in *your *code, not in Wicket.
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
On Wed, May 19, 2010 at 4:30 PM, Adriano dos Santos Fernandes adrian...@gmail.com wrote: I do not think the application object is general enough to be passed implicitly to threads. Now, this is a valid argument! I agree wholeheartedly that Application.get() shouldn't be accessible to child threads by default. But, it's just my opinion about how to design applications and deal with thread issues, not some fundamental flaw. Anyway, the InheritableThreadLocal doesn't seem to cause any memory leak issue that wouldn't already occur due threads running amok. If the maintainers don't see a problem with it, I think this is an non-issue.
Re: [vote] Release Wicket 1.4.9
On 19/05/2010 16:37, tetsuo wrote: On Wed, May 19, 2010 at 4:20 PM, Adriano dos Santos Fernandes adrian...@gmail.com wrote: I can't, however, do it now. But I would have no reason to do it knowing some folks just consider a normal thing restart the container to update the application. So if Wicket devs are in this way, I could write no quickstart to convince. Cara, agora você está sendo um grande babacão... Sorry, but I'm not a child (as you seems to be) to have a flame war with you. Adriano
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
So, your itch or usecase is that you're trying to use Wicket in a way it was not intended. On Wed, May 19, 2010 at 3:48 PM, Adriano dos Santos Fernandes adrian...@gmail.com wrote: On 19/05/2010 16:36, James Carman wrote: What itch are we trying to scratch, here, anyway? When do folks need access to the Application object outside of a request thread? What is the usecase? I have a piece of code that runs in a timer, and renders a page to a string to be sent via email. So I do need the app object to make a session. I do catch it from a static field. The code (that is probably inspired in something in the wiki) is not very clear, needing mock objects. I do think Wicket could have a way to simplify it, but not introducing leaks that's know to happen with Java system classes way of work, as I shown. Adriano
Re: [vote] Release Wicket 1.4.9
On Wed, May 19, 2010 at 2:20 PM, Adriano dos Santos Fernandes adrian...@gmail.com wrote: On 19/05/2010 15:57, Jeremy Thomerson wrote: Here's how you create a quickstart: http://www.jeremythomerson.com/blog/2008/11/wicket-quickstart-tutorial/ If you find that there is a bug, you zip your quickstart directory and attach it to a JIRA issue. Then we fix it and build a new release and start a new vote (if the bug is serious enough). I know, I know... For example, the ones I have in Jira and got closed without analyze, or the ones I attached with fixes. Which ones? We don't close any without looking at them first. Please bring these to our attention on another thread - not this vote thread. I can't, however, do it now. But I would have no reason to do it knowing some folks just consider a normal thing restart the container to update the application. So if Wicket devs are in this way, I could write no quickstart to convince. Look, none of the Wicket devs have said you have to restart the container. All we have said is that you are complaining that I think there might be a bug, and have shown no proof. As many people have said - on this thread and your other one - the bug will be YOURS if YOU are leaving threads running past a redeploy. So, we still have no proof that there is a bug. Guice, by its use of thread locals, and considering that Java thread locals are not ideal, have the same type of bug. They could solve it with an API to close a thing, but they don't. They could ship some fancy classes that may work (accordingly to old @crazybob says) in some cases but they also didn't. So if you redeploy an app with Guice in Tomcat, it logs about a thread local leak. I don't care what Guice does. However, I do care if there's a bug in Wicket. If there is, I'll try to fix it. But you must show the bug - I'm not going to do free development to prove that your thesis is correct. Finally, please take this off the vote thread. This thread is for voting whether or not the release should be released. Our devs spend hours building these releases, and then we open a vote, and someone always whines that they didn't get what they wanted. But, if someone brings up VALID bug and SHOWS SOME PROOF that it is really a bug (or it's blatantly obvious), then we again spend hours building a new release. But, again, I don't work for you, and I will not spend hours of my day proving your thesis. I've already spent enough time just trying to ask you nicely to submit something that proves that there's a bug in our code. The only thing we've proven so far is that, *yes* a bad developer can write bad code that results in bad consequences. Jeremy Thomerson
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
To clarify this, I use Application.set and Application.unset there. Although public, they're not part of Wicket public API. So IMO, the initial issue would be better done making these methods part of the public API. And then people all over will have memory leak issues because they'll forget a finally block. We tell you those are not part of the public API because you should *not* use them. If you want to use them that way, then fine, but you have to deal with the consequences - not us.
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
On 19/05/2010 17:03, Jeremy Thomerson wrote: To clarify this, I use Application.set and Application.unset there. Although public, they're not part of Wicket public API. So IMO, the initial issue would be better done making these methods part of the public API. And then people all over will have memory leak issues because they'll forget a finally block. Well, forgetting to unset it would not leak any more than have it implicitly set like it's going to be. And I do think forgetting this is developer fault. What you all do not want to understand is what I said about Java library spawning its own threads, and that is not documented, as its for cleanup in the case I shown. Adriano
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
Will the inheritance of the application really work correctly? For pooled threads that are created at application startup, the threadlocal will be null, because the parent thread is the thread that starts the container. For threads that are created within the context of the request thread, they will get the current application object, which would be fine if that thread executes and finishes. But, for threads that are going to be reused (executor threads in a pool), they will see the original application object because the value is set at thread creation time. If you have multiple wicket filters in the same context, that could be incorrect, meaning a request thread for a different application submitted a task to be executed. On May 19, 2010 4:13 PM, Adriano dos Santos Fernandes adrian...@gmail.com wrote: On 19/05/2010 17:03, Jeremy Thomerson wrote: To clarify this, I use Application.set and App... Well, forgetting to unset it would not leak any more than have it implicitly set like it's going to be. And I do think forgetting this is developer fault. What you all do not want to understand is what I said about Java library spawning its own threads, and that is not documented, as its for cleanup in the case I shown. Adriano
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
I wondered about this too: would this work with a job framework like Quartz? The thread is not started in a wicket context, but by the thing that quartz is managing. Therefore the inherited thing would not work and the Application would not be set. Martijn On Wed, May 19, 2010 at 11:41 PM, James Carman ja...@carmanconsulting.com wrote: Will the inheritance of the application really work correctly? For pooled threads that are created at application startup, the threadlocal will be null, because the parent thread is the thread that starts the container. For threads that are created within the context of the request thread, they will get the current application object, which would be fine if that thread executes and finishes. But, for threads that are going to be reused (executor threads in a pool), they will see the original application object because the value is set at thread creation time. If you have multiple wicket filters in the same context, that could be incorrect, meaning a request thread for a different application submitted a task to be executed. On May 19, 2010 4:13 PM, Adriano dos Santos Fernandes adrian...@gmail.com wrote: On 19/05/2010 17:03, Jeremy Thomerson wrote: To clarify this, I use Application.set and App... Well, forgetting to unset it would not leak any more than have it implicitly set like it's going to be. And I do think forgetting this is developer fault. What you all do not want to understand is what I said about Java library spawning its own threads, and that is not documented, as its for cleanup in the case I shown. Adriano -- Become a Wicket expert, learn from the best: http://wicketinaction.com Apache Wicket 1.4 increases type safety for web applications Get it now: http://www.apache.org/dyn/closer.cgi/wicket/1.4.8
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
why would a request thread start your executor pool? the pool would most likely be started from a context listener which uses a separate thread. the usecase for the inheritable is for short-lived threads started from a request thread. -igor On Wed, May 19, 2010 at 2:41 PM, James Carman ja...@carmanconsulting.com wrote: Will the inheritance of the application really work correctly? For pooled threads that are created at application startup, the threadlocal will be null, because the parent thread is the thread that starts the container. For threads that are created within the context of the request thread, they will get the current application object, which would be fine if that thread executes and finishes. But, for threads that are going to be reused (executor threads in a pool), they will see the original application object because the value is set at thread creation time. If you have multiple wicket filters in the same context, that could be incorrect, meaning a request thread for a different application submitted a task to be executed. On May 19, 2010 4:13 PM, Adriano dos Santos Fernandes adrian...@gmail.com wrote: On 19/05/2010 17:03, Jeremy Thomerson wrote: To clarify this, I use Application.set and App... Well, forgetting to unset it would not leak any more than have it implicitly set like it's going to be. And I do think forgetting this is developer fault. What you all do not want to understand is what I said about Java library spawning its own threads, and that is not documented, as its for cleanup in the case I shown. Adriano
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
thinking about this more, in 1.5 these short-lived threads will not even be needed. most often you need to start a new thread so you have a properly configured wicket environment which depends on threadlocals. this environment is usually configured by wicket tester which sets up mock session, request cycle threadlocal. in 1.5 you can simply push wicket's threadcontext and replace it with a separate temporary one and when you are done simply pop it off. in 1.4 because all these threadlocals are seperate it is massier to manage them, thus its simply easier to do the work in another thread. -igor On Wed, May 19, 2010 at 2:52 PM, Igor Vaynberg igor.vaynb...@gmail.com wrote: why would a request thread start your executor pool? the pool would most likely be started from a context listener which uses a separate thread. the usecase for the inheritable is for short-lived threads started from a request thread. -igor On Wed, May 19, 2010 at 2:41 PM, James Carman ja...@carmanconsulting.com wrote: Will the inheritance of the application really work correctly? For pooled threads that are created at application startup, the threadlocal will be null, because the parent thread is the thread that starts the container. For threads that are created within the context of the request thread, they will get the current application object, which would be fine if that thread executes and finishes. But, for threads that are going to be reused (executor threads in a pool), they will see the original application object because the value is set at thread creation time. If you have multiple wicket filters in the same context, that could be incorrect, meaning a request thread for a different application submitted a task to be executed. On May 19, 2010 4:13 PM, Adriano dos Santos Fernandes adrian...@gmail.com wrote: On 19/05/2010 17:03, Jeremy Thomerson wrote: To clarify this, I use Application.set and App... Well, forgetting to unset it would not leak any more than have it implicitly set like it's going to be. And I do think forgetting this is developer fault. What you all do not want to understand is what I said about Java library spawning its own threads, and that is not documented, as its for cleanup in the case I shown. Adriano
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
If you where using threads in your application
Then i would do it this way:
Your WebApplication class has a method:
getExecutor() that returns a ThreadPoolExecutor
That threadpoolexecutor (that you extend) overrides 2 methods
protected void beforeExecute(Thread t, Runnable r) { }
that sets the thread locals (so the application instance that has the
executor) and
protected void afterExecute(Runnable r, Throwable t) { }
to release all thread locals.
this way you use a pool (way better to control your web application) and all
the resources you need are set just before and release right after.
johanm
On Wed, May 19, 2010 at 23:41, James Carman ja...@carmanconsulting.comwrote:
Will the inheritance of the application really work correctly? For pooled
threads that are created at application startup, the threadlocal will be
null, because the parent thread is the thread that starts the container.
For threads that are created within the context of the request thread, they
will get the current application object, which would be fine if that thread
executes and finishes. But, for threads that are going to be reused
(executor threads in a pool), they will see the original application object
because the value is set at thread creation time. If you have multiple
wicket filters in the same context, that could be incorrect, meaning a
request thread for a different application submitted a task to be executed.
On May 19, 2010 4:13 PM, Adriano dos Santos Fernandes
adrian...@gmail.com
wrote:
On 19/05/2010 17:03, Jeremy Thomerson wrote:
To clarify this, I use Application.set and App...
Well, forgetting to unset it would not leak any more than have it
implicitly
set like it's going to be. And I do think forgetting this is developer
fault.
What you all do not want to understand is what I said about Java library
spawning its own threads, and that is not documented, as its for cleanup in
the case I shown.
Adriano
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
Sure this might work, but then again you wouldn't need the
InheritableThreadLocal for this. The question is, does the
InheritableThreadLocal really solve anything? Is it really addressing
the problem? Or, would you have to do code like this to manage it
properly anyway? And, if so, then why implement the
InheritableThreadLocal, especially since we've shown that it will fail
in more cases than it will work?
On Wed, May 19, 2010 at 6:28 PM, Johan Compagner jcompag...@gmail.com wrote:
If you where using threads in your application
Then i would do it this way:
Your WebApplication class has a method:
getExecutor() that returns a ThreadPoolExecutor
That threadpoolexecutor (that you extend) overrides 2 methods
protected void beforeExecute(Thread t, Runnable r) { }
that sets the thread locals (so the application instance that has the
executor) and
protected void afterExecute(Runnable r, Throwable t) { }
to release all thread locals.
this way you use a pool (way better to control your web application) and all
the resources you need are set just before and release right after.
johanm
On Wed, May 19, 2010 at 23:41, James Carman ja...@carmanconsulting.comwrote:
Will the inheritance of the application really work correctly? For pooled
threads that are created at application startup, the threadlocal will be
null, because the parent thread is the thread that starts the container.
For threads that are created within the context of the request thread, they
will get the current application object, which would be fine if that thread
executes and finishes. But, for threads that are going to be reused
(executor threads in a pool), they will see the original application object
because the value is set at thread creation time. If you have multiple
wicket filters in the same context, that could be incorrect, meaning a
request thread for a different application submitted a task to be executed.
On May 19, 2010 4:13 PM, Adriano dos Santos Fernandes
adrian...@gmail.com
wrote:
On 19/05/2010 17:03, Jeremy Thomerson wrote:
To clarify this, I use Application.set and App...
Well, forgetting to unset it would not leak any more than have it
implicitly
set like it's going to be. And I do think forgetting this is developer
fault.
What you all do not want to understand is what I said about Java library
spawning its own threads, and that is not documented, as its for cleanup in
the case I shown.
Adriano
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
It solves this problem, which is specifically why it was requested:
onClickOrSomethingSimilar() {
new Thread(new Runnable() {
void run() {
doSomethingWith(Application.get());
}
}).start();
}
--
Jeremy Thomerson
http://www.wickettraining.com
On Wed, May 19, 2010 at 6:28 PM, James Carman ja...@carmanconsulting.comwrote:
Sure this might work, but then again you wouldn't need the
InheritableThreadLocal for this. The question is, does the
InheritableThreadLocal really solve anything? Is it really addressing
the problem? Or, would you have to do code like this to manage it
properly anyway? And, if so, then why implement the
InheritableThreadLocal, especially since we've shown that it will fail
in more cases than it will work?
On Wed, May 19, 2010 at 6:28 PM, Johan Compagner jcompag...@gmail.com
wrote:
If you where using threads in your application
Then i would do it this way:
Your WebApplication class has a method:
getExecutor() that returns a ThreadPoolExecutor
That threadpoolexecutor (that you extend) overrides 2 methods
protected void beforeExecute(Thread t, Runnable r) { }
that sets the thread locals (so the application instance that has the
executor) and
protected void afterExecute(Runnable r, Throwable t) { }
to release all thread locals.
this way you use a pool (way better to control your web application) and
all
the resources you need are set just before and release right after.
johanm
On Wed, May 19, 2010 at 23:41, James Carman ja...@carmanconsulting.com
wrote:
Will the inheritance of the application really work correctly? For
pooled
threads that are created at application startup, the threadlocal will be
null, because the parent thread is the thread that starts the container.
For threads that are created within the context of the request thread,
they
will get the current application object, which would be fine if that
thread
executes and finishes. But, for threads that are going to be reused
(executor threads in a pool), they will see the original application
object
because the value is set at thread creation time. If you have multiple
wicket filters in the same context, that could be incorrect, meaning a
request thread for a different application submitted a task to be
executed.
On May 19, 2010 4:13 PM, Adriano dos Santos Fernandes
adrian...@gmail.com
wrote:
On 19/05/2010 17:03, Jeremy Thomerson wrote:
To clarify this, I use Application.set and App...
Well, forgetting to unset it would not leak any more than have it
implicitly
set like it's going to be. And I do think forgetting this is developer
fault.
What you all do not want to understand is what I said about Java library
spawning its own threads, and that is not documented, as its for cleanup
in
the case I shown.
Adriano
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
It solves that one particular usecase, but I doubt folks would be
starting threads like that. Most folks, if they're smart, would use a
thread pool for something like that. For the pooled thread case, it
doesn't work.
On Wed, May 19, 2010 at 9:29 PM, Jeremy Thomerson
jer...@wickettraining.com wrote:
It solves this problem, which is specifically why it was requested:
onClickOrSomethingSimilar() {
new Thread(new Runnable() {
void run() {
doSomethingWith(Application.get());
}
}).start();
}
--
Jeremy Thomerson
http://www.wickettraining.com
On Wed, May 19, 2010 at 6:28 PM, James Carman
ja...@carmanconsulting.comwrote:
Sure this might work, but then again you wouldn't need the
InheritableThreadLocal for this. The question is, does the
InheritableThreadLocal really solve anything? Is it really addressing
the problem? Or, would you have to do code like this to manage it
properly anyway? And, if so, then why implement the
InheritableThreadLocal, especially since we've shown that it will fail
in more cases than it will work?
On Wed, May 19, 2010 at 6:28 PM, Johan Compagner jcompag...@gmail.com
wrote:
If you where using threads in your application
Then i would do it this way:
Your WebApplication class has a method:
getExecutor() that returns a ThreadPoolExecutor
That threadpoolexecutor (that you extend) overrides 2 methods
protected void beforeExecute(Thread t, Runnable r) { }
that sets the thread locals (so the application instance that has the
executor) and
protected void afterExecute(Runnable r, Throwable t) { }
to release all thread locals.
this way you use a pool (way better to control your web application) and
all
the resources you need are set just before and release right after.
johanm
On Wed, May 19, 2010 at 23:41, James Carman ja...@carmanconsulting.com
wrote:
Will the inheritance of the application really work correctly? For
pooled
threads that are created at application startup, the threadlocal will be
null, because the parent thread is the thread that starts the container.
For threads that are created within the context of the request thread,
they
will get the current application object, which would be fine if that
thread
executes and finishes. But, for threads that are going to be reused
(executor threads in a pool), they will see the original application
object
because the value is set at thread creation time. If you have multiple
wicket filters in the same context, that could be incorrect, meaning a
request thread for a different application submitted a task to be
executed.
On May 19, 2010 4:13 PM, Adriano dos Santos Fernandes
adrian...@gmail.com
wrote:
On 19/05/2010 17:03, Jeremy Thomerson wrote:
To clarify this, I use Application.set and App...
Well, forgetting to unset it would not leak any more than have it
implicitly
set like it's going to be. And I do think forgetting this is developer
fault.
What you all do not want to understand is what I said about Java library
spawning its own threads, and that is not documented, as its for cleanup
in
the case I shown.
Adriano
RE: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
Agreed - I use pools for stuff like this. But, it's not broken for pools -
nothing functionally changed, right?
Jeremy Thomerson
http://www.wickettraining.com
-- sent from a wireless device
-Original Message-
From: James Carman ja...@carmanconsulting.com
Sent: Wednesday, May 19, 2010 9:28 PM
To: dev@wicket.apache.org
Subject: Re: WICKET-2846 - Store Application in InheritableThreadLocal instead
of ThreadLocal
It solves that one particular usecase, but I doubt folks would be
starting threads like that. Most folks, if they're smart, would use a
thread pool for something like that. For the pooled thread case, it
doesn't work.
On Wed, May 19, 2010 at 9:29 PM, Jeremy Thomerson
jer...@wickettraining.com wrote:
It solves this problem, which is specifically why it was requested:
onClickOrSomethingSimilar() {
new Thread(new Runnable() {
void run() {
doSomethingWith(Application.get());
}
}).start();
}
--
Jeremy Thomerson
http://www.wickettraining.com
On Wed, May 19, 2010 at 6:28 PM, James Carman
ja...@carmanconsulting.comwrote:
Sure this might work, but then again you wouldn't need the
InheritableThreadLocal for this. The question is, does the
InheritableThreadLocal really solve anything? Is it really addressing
the problem? Or, would you have to do code like this to manage it
properly anyway? And, if so, then why implement the
InheritableThreadLocal, especially since we've shown that it will fail
in more cases than it will work?
On Wed, May 19, 2010 at 6:28 PM, Johan Compagner jcompag...@gmail.com
wrote:
If you where using threads in your application
Then i would do it this way:
Your WebApplication class has a method:
getExecutor() that returns a ThreadPoolExecutor
That threadpoolexecutor (that you extend) overrides 2 methods
protected void beforeExecute(Thread t, Runnable r) { }
that sets the thread locals (so the application instance that has the
executor) and
protected void afterExecute(Runnable r, Throwable t) { }
to release all thread locals.
this way you use a pool (way better to control your web application) and
all
the resources you need are set just before and release right after.
johanm
On Wed, May 19, 2010 at 23:41, James Carman ja...@carmanconsulting.com
wrote:
Will the inheritance of the application really work correctly? For
pooled
threads that are created at application startup, the threadlocal will be
null, because the parent thread is the thread that starts the container.
For threads that are created within the context of the request thread,
they
will get the current application object, which would be fine if that
thread
executes and finishes. But, for threads that are going to be reused
(executor threads in a pool), they will see the original application
object
because the value is set at thread creation time. If you have multiple
wicket filters in the same context, that could be incorrect, meaning a
request thread for a different application submitted a task to be
executed.
On May 19, 2010 4:13 PM, Adriano dos Santos Fernandes
adrian...@gmail.com
wrote:
On 19/05/2010 17:03, Jeremy Thomerson wrote:
To clarify this, I use Application.set and App...
Well, forgetting to unset it would not leak any more than have it
implicitly
set like it's going to be. And I do think forgetting this is developer
fault.
What you all do not want to understand is what I said about Java library
spawning its own threads, and that is not documented, as its for cleanup
in
the case I shown.
Adriano
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal
onClickOrSomethingSimilar() {
final Application app = Application.get();
new Thread(new Runnable() {
void run() {
doSomethingWith(app);
}
}).start();
}
On Wed, May 19, 2010 at 10:29 PM, Jeremy Thomerson
jer...@wickettraining.com wrote:
It solves this problem, which is specifically why it was requested:
onClickOrSomethingSimilar() {
new Thread(new Runnable() {
void run() {
doSomethingWith(Application.get());
}
}).start();
}
--
Jeremy Thomerson
http://www.wickettraining.com
On Wed, May 19, 2010 at 6:28 PM, James Carman ja...@carmanconsulting.com
wrote:
Sure this might work, but then again you wouldn't need the
InheritableThreadLocal for this. The question is, does the
InheritableThreadLocal really solve anything? Is it really addressing
the problem? Or, would you have to do code like this to manage it
properly anyway? And, if so, then why implement the
InheritableThreadLocal, especially since we've shown that it will fail
in more cases than it will work?
On Wed, May 19, 2010 at 6:28 PM, Johan Compagner jcompag...@gmail.com
wrote:
If you where using threads in your application
Then i would do it this way:
Your WebApplication class has a method:
getExecutor() that returns a ThreadPoolExecutor
That threadpoolexecutor (that you extend) overrides 2 methods
protected void beforeExecute(Thread t, Runnable r) { }
that sets the thread locals (so the application instance that has the
executor) and
protected void afterExecute(Runnable r, Throwable t) { }
to release all thread locals.
this way you use a pool (way better to control your web application)
and
all
the resources you need are set just before and release right after.
johanm
On Wed, May 19, 2010 at 23:41, James Carman
ja...@carmanconsulting.com
wrote:
Will the inheritance of the application really work correctly? For
pooled
threads that are created at application startup, the threadlocal will
be
null, because the parent thread is the thread that starts the
container.
For threads that are created within the context of the request thread,
they
will get the current application object, which would be fine if that
thread
executes and finishes. But, for threads that are going to be reused
(executor threads in a pool), they will see the original application
object
because the value is set at thread creation time. If you have
multiple
wicket filters in the same context, that could be incorrect, meaning a
request thread for a different application submitted a task to be
executed.
On May 19, 2010 4:13 PM, Adriano dos Santos Fernandes
adrian...@gmail.com
wrote:
On 19/05/2010 17:03, Jeremy Thomerson wrote:
To clarify this, I use Application.set and App...
Well, forgetting to unset it would not leak any more than have it
implicitly
set like it's going to be. And I do think forgetting this is developer
fault.
What you all do not want to understand is what I said about Java
library
spawning its own threads, and that is not documented, as its for
cleanup
in
the case I shown.
Adriano
