You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Johan Compagner <jc...@gmail.com> on 2010/05/19 21:16:50 UTC
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: WICKET-2846 - Store Application in InheritableThreadLocal instead
of ThreadLocal
Posted by James Carman <ja...@carmanconsulting.com>.
On Thu, May 20, 2010 at 7:08 AM, Adriano dos Santos Fernandes
<ad...@gmail.com> 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
Posted by Adriano dos Santos Fernandes <ad...@gmail.com>.
On 20/05/2010 08:02, James Carman wrote:
> On Thu, May 20, 2010 at 6:59 AM, Adriano dos Santos Fernandes
> <ad...@gmail.com> 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 one).
...
> Even "if" you start
> the threads in the pool during one particular request cycle, what
> happens when someone else submits a task to be run by the executor?
> It will also use this original application, but it may not necessarily
> be the application they're interested in. You can have multiple
> WicketFilter's (and thus applications) in the same webapp.
>
I just don't get if (and what/why) you agree or disagree with what I've
said...
Adriano
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead
of ThreadLocal
Posted by James Carman <ja...@carmanconsulting.com>.
On Thu, May 20, 2010 at 6:59 AM, Adriano dos Santos Fernandes
<ad...@gmail.com> 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 one). Even "if" you start
the threads in the pool during one particular request cycle, what
happens when someone else submits a task to be run by the executor?
It will also use this original application, but it may not necessarily
be the application they're interested in. You can have multiple
WicketFilter's (and thus applications) in the same webapp.
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead
of ThreadLocal
Posted by Adriano dos Santos Fernandes <ad...@gmail.com>.
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.
>
I do not know Quartz, but I suppose a lot of problem can arrive if the
library is shared by all applications.
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.
Adriano
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead
of ThreadLocal
Posted by Martijn Dashorst <ma...@gmail.com>.
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" <ad...@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
Posted by James Carman <ja...@carmanconsulting.com>.
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
<je...@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 <jc...@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 <james@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" <
>> >> adrianosf@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
Posted by Juliano Viana <ju...@logicstyle.com>.
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 break applications that are not already broken.
I have found a real case where this breaks an innocent application:
redeploying an application based on Wicket 1.4.9 on Glassfish 3.0.1 causes
a memory leak due to the use of InheritableThreadLocal.
The problem is that when the application accesses a JDBC resource for the
first time, Glassfish lazily starts a timer (connector-timer-proxy) that has
an associated thread. This timer is started from the web request processing
thread. This thread never dies, and inherits a reference to the Wicket
Application object.
This only happens on redeployments, but it really hurts development as you
keep having to restart Glassfish due to OOM exceptions.
Removing the InheritableThreadLocal resolves the issue completely and makes
development really smooth again.
So if you are using Wicket 1.4.9 with Glassfish v3 you should consider
patching it until a new Wicket release is out.
Regards,
- Juliano
On Thu, May 20, 2010 at 2:46 PM, tetsuo <ro...@gmail.com> wrote:
> On Thu, May 20, 2010 at 10:29 AM, Johan Compagner <jcompagner@gmail.com
> >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 correctly.
>
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead
of ThreadLocal
Posted by tetsuo <ro...@gmail.com>.
On Thu, May 20, 2010 at 10:29 AM, Johan Compagner <jc...@gmail.com>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 correctly.
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead
of ThreadLocal
Posted by Johan Compagner <jc...@gmail.com>.
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
is exactly the same kind of leakage as using InheritableThreadLocal
On Thu, May 20, 2010 at 12:38, Adriano dos Santos Fernandes <
adrianosf@gmail.com> wrote:
> 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 said about have a WicketThread class, or "publish"
> Application.set/clear for users do this with more safety.
>
>
> Adriano
>
>
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead
of ThreadLocal
Posted by Adriano dos Santos Fernandes <ad...@gmail.com>.
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 said about have a WicketThread class, or "publish"
Application.set/clear for users do this with more safety.
Adriano
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead
of ThreadLocal
Posted by tetsuo <ro...@gmail.com>.
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 <
jeremy@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 <james@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 <jc...@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 <
> james@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" <
> > >> adrianosf@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
Posted by Jeremy Thomerson <je...@wickettraining.com>.
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 <jc...@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 <james@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" <
> >> adrianosf@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
Posted by James Carman <ja...@carmanconsulting.com>.
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 <jc...@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" <
>> adrianosf@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
Posted by Johan Compagner <jc...@gmail.com>.
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" <
> adrianosf@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
Posted by Igor Vaynberg <ig...@gmail.com>.
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 <ig...@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" <ad...@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
Posted by Igor Vaynberg <ig...@gmail.com>.
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" <ad...@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
Posted by James Carman <ja...@carmanconsulting.com>.
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" <ad...@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
Posted by Michael Mosmann <mi...@mosmann.de>.
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 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.
I use the BaseWicketTester-Class for this kind of stuff..
http://www.wicket-praxis.de/blog/2009/12/01/sending-html-email-from-wicket-app/
mm:)
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead
of ThreadLocal
Posted by Adriano dos Santos Fernandes <ad...@gmail.com>.
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
Posted by Jeremy Thomerson <je...@wickettraining.com>.
>
>
> 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
Posted by Adriano dos Santos Fernandes <ad...@gmail.com>.
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, 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.
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.
Adriano
Re: WICKET-2846 - Store Application in InheritableThreadLocal instead
of ThreadLocal
Posted by James Carman <ja...@carmanconsulting.com>.
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
<ad...@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: WICKET-2846 - Store Application in InheritableThreadLocal instead
of ThreadLocal
Posted by Adriano dos Santos Fernandes <ad...@gmail.com>.
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: WICKET-2846 - Store Application in InheritableThreadLocal instead
of ThreadLocal
Posted by James Carman <ja...@carmanconsulting.com>.
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
<ad...@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: WICKET-2846 - Store Application in InheritableThreadLocal instead
of ThreadLocal
Posted by tetsuo <ro...@gmail.com>.
On Wed, May 19, 2010 at 4:30 PM, Adriano dos Santos Fernandes <
adrianosf@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: WICKET-2846 - Store Application in InheritableThreadLocal instead
of ThreadLocal
Posted by Adriano dos Santos Fernandes <ad...@gmail.com>.
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