Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-07-14 Thread Juliano Viana
Hi everyone, I know this issue has already been debated and that a decision was made to revert this change in a future version of Wicket. However, the discussions about this issue were centered on the fact starting threads in web applications is not a good idea anyway, and hence this would not bre

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-20 Thread tetsuo
On Thu, May 20, 2010 at 10:29 AM, Johan Compagner wrote: > But using > > final Application app = Application.get() > // start thread > > is exactly the same kind of leakage as using InheritableThreadLocal > > Exactly, the bug is not in Wicket, it's in the application, which doesn't manage threads

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-20 Thread Johan Compagner
i still dont see your solution about the wicket thread class. What should that one do??? give an example. The best solution is to use a threadpool like a described above. And yes that InheritableThreadLocal isnt needed then. But using final Application app = Application.get() // start thread i

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-20 Thread James Carman
On Thu, May 20, 2010 at 7:08 AM, Adriano dos Santos Fernandes wrote: > I just don't get if (and what/why) you agree or disagree with what I've > said... > Apparently I was agreeing with you. I guess I didn't read your post closely enough. :) NEED MORE COFFEE!

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-20 Thread Adriano dos Santos Fernandes
On 20/05/2010 08:02, James Carman wrote: On Thu, May 20, 2010 at 6:59 AM, Adriano dos Santos Fernandes wrote: If its thread pool starts during a request, all of its created child threads will have a reference to the initial application, even when running jobs of others applications.

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-20 Thread James Carman
On Thu, May 20, 2010 at 6:59 AM, Adriano dos Santos Fernandes wrote: > If its thread pool starts during a request, all of its created child threads > will have a reference to the initial application, even when running jobs of > others applications. > That's a big "if" (and usually an incorrect on

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-20 Thread Adriano dos Santos Fernandes
On 19/05/2010 18:46, Martijn Dashorst wrote: 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.

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-20 Thread Adriano dos Santos Fernandes
On 19/05/2010 22:29, Jeremy Thomerson wrote: It solves this problem, which is specifically why it was requested: onClickOrSomethingSimilar() { new Thread(new Runnable() { void run() { doSomethingWith(Application.get()); } }).start(); } That's what I s

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-20 Thread Michael Mosmann
Am Mittwoch, den 19.05.2010, 16:48 -0300 schrieb Adriano dos Santos Fernandes: > 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

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread tetsuo
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

RE: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread Jeremy Thomerson
: 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

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread James Carman
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 wrote: > It solves this pro

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread Jeremy Thomerson
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, 2

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread James Carman
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 imp

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread Johan Compagner
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

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread Igor Vaynberg
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, reque

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread Igor Vaynberg
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 wrote: >

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread Martijn Dashorst
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

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread James Carman
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 wi

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread Adriano dos Santos Fernandes
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

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread Jeremy Thomerson
> > > 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 the

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread James Carman
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 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

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread Adriano dos Santos Fernandes
On 19/05/2010 16:48, Adriano dos Santos Fernandes 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,

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread tetsuo
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 ch

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread Adriano dos Santos Fernandes
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.

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread James Carman
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 wrote: > On 19/05/2010 16:16, Johan Compagner wrote: >> >> >> - Original mess

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread Adriano dos Santos Fernandes
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

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread Johan Compagner
- 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 th

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread tetsuo
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 und

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread tetsuo
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 caref

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread Adriano dos Santos Fernandes
On 19/05/2010 14:35, James Carman wrote: On Wed, May 19, 2010 at 11:56 AM, Adriano dos Santos Fernandes 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

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread James Carman
On Wed, May 19, 2010 at 11:56 AM, Adriano dos Santos Fernandes 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 thr

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread Adriano dos Santos Fernandes
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 w

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread Alex Objelean
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 cont

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread Adriano dos Santos Fernandes
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 thr

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread Adriano dos Santos Fernandes
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.

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread James Carman
Use JRebel! Problem solved. :) On Wed, May 19, 2010 at 10:03 AM, Adriano dos Santos Fernandes wrote: > 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

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread Alex Objelean
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://apach

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread Adriano dos Santos Fernandes
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 somet

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread James Carman
Why would the application object itself need to be garbage collected? On Wed, May 19, 2010 at 8:53 AM, Adriano dos Santos Fernandes 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

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread Adriano dos Santos Fernandes
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 create

Re: WICKET-2846 - Store Application in InheritableThreadLocal instead of ThreadLocal

2010-05-19 Thread Alex Objelean
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-Inheritabl