You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Jeremy Thomerson <je...@wickettraining.com> on 2010/05/22 09:22:35 UTC
[vote] Revert WICKET-2846
As you may have noticed over the past couple of days (ha), there has been
quite a bit of discussion over what seemed at the time like a very trivial
change in WICKET-2846 [*]. The end result is that it does not break any
existing applications that don't already have a bug. However, if the
application is already faulty in the way it uses threads (perhaps even due
to a bug within Java related to cleaning up Java2D threads), then the change
in WICKET-2846 can exacerbate the issue. On the flip side, the
"enhancement" in WICKET-2846 is of very minor value to only a very small
subset of cases that should rarely be used since they would not be
considered best practices. And, if it were reverted, there would be fairly
easy ways to get the same functionality without this change.
So, I leave it to the community to vote on this. Because I feel neither +1
or -1 on this issue, my (binding) vote will go to whatever non-binding
majority wins. So, in this case, *every vote counts* - even if your vote is
typically non-binding.
As one last comment, please: we don't need any more long-running discussions
or diatribes on this. We already know this issue intimately. Please simply
vote, and if you must, provide a simple reason why you're voting the way you
are.
[ ] +1 - revert WICKET-2846 in the next release (in other words, get rid of
the InheritableThreadLocal)
[ ] -1 - leave everything exactly as it is (in other words, keep the
InheritableThreadLocal)
[*] - https://issues.apache.org/jira/browse/WICKET-2846
Best regards,
--
Jeremy Thomerson
http://www.wickettraining.com
Re: [vote] Revert WICKET-2846
Posted by Martijn Dashorst <ma...@gmail.com>.
+1 revert.
Martijn
On Sat, May 22, 2010 at 9:22 AM, Jeremy Thomerson
<je...@wickettraining.com> wrote:
> As you may have noticed over the past couple of days (ha), there has been
> quite a bit of discussion over what seemed at the time like a very trivial
> change in WICKET-2846 [*]. The end result is that it does not break any
> existing applications that don't already have a bug. However, if the
> application is already faulty in the way it uses threads (perhaps even due
> to a bug within Java related to cleaning up Java2D threads), then the change
> in WICKET-2846 can exacerbate the issue. On the flip side, the
> "enhancement" in WICKET-2846 is of very minor value to only a very small
> subset of cases that should rarely be used since they would not be
> considered best practices. And, if it were reverted, there would be fairly
> easy ways to get the same functionality without this change.
>
> So, I leave it to the community to vote on this. Because I feel neither +1
> or -1 on this issue, my (binding) vote will go to whatever non-binding
> majority wins. So, in this case, *every vote counts* - even if your vote is
> typically non-binding.
>
> As one last comment, please: we don't need any more long-running discussions
> or diatribes on this. We already know this issue intimately. Please simply
> vote, and if you must, provide a simple reason why you're voting the way you
> are.
>
> [ ] +1 - revert WICKET-2846 in the next release (in other words, get rid of
> the InheritableThreadLocal)
> [ ] -1 - leave everything exactly as it is (in other words, keep the
> InheritableThreadLocal)
>
> [*] - https://issues.apache.org/jira/browse/WICKET-2846
>
> Best regards,
>
> --
> Jeremy Thomerson
> http://www.wickettraining.com
>
--
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: [vote] Revert WICKET-2846
Posted by James Carman <ja...@carmanconsulting.com>.
+1 - revert
On Sat, May 22, 2010 at 3:22 AM, Jeremy Thomerson
<je...@wickettraining.com> wrote:
> As you may have noticed over the past couple of days (ha), there has been
> quite a bit of discussion over what seemed at the time like a very trivial
> change in WICKET-2846 [*]. The end result is that it does not break any
> existing applications that don't already have a bug. However, if the
> application is already faulty in the way it uses threads (perhaps even due
> to a bug within Java related to cleaning up Java2D threads), then the change
> in WICKET-2846 can exacerbate the issue. On the flip side, the
> "enhancement" in WICKET-2846 is of very minor value to only a very small
> subset of cases that should rarely be used since they would not be
> considered best practices. And, if it were reverted, there would be fairly
> easy ways to get the same functionality without this change.
>
> So, I leave it to the community to vote on this. Because I feel neither +1
> or -1 on this issue, my (binding) vote will go to whatever non-binding
> majority wins. So, in this case, *every vote counts* - even if your vote is
> typically non-binding.
>
> As one last comment, please: we don't need any more long-running discussions
> or diatribes on this. We already know this issue intimately. Please simply
> vote, and if you must, provide a simple reason why you're voting the way you
> are.
>
> [ ] +1 - revert WICKET-2846 in the next release (in other words, get rid of
> the InheritableThreadLocal)
> [ ] -1 - leave everything exactly as it is (in other words, keep the
> InheritableThreadLocal)
>
> [*] - https://issues.apache.org/jira/browse/WICKET-2846
>
> Best regards,
>
> --
> Jeremy Thomerson
> http://www.wickettraining.com
>
Re: [vote] Revert WICKET-2846
Posted by Jeremy Thomerson <je...@wickettraining.com>.
On Sat, May 22, 2010 at 2:22 AM, Jeremy Thomerson <jeremy@wickettraining.com
> wrote:
> As you may have noticed over the past couple of days (ha), there has been
> quite a bit of discussion over what seemed at the time like a very trivial
> change in WICKET-2846 [*]. The end result is that it does not break any
> existing applications that don't already have a bug. However, if the
> application is already faulty in the way it uses threads (perhaps even due
> to a bug within Java related to cleaning up Java2D threads), then the change
> in WICKET-2846 can exacerbate the issue. On the flip side, the
> "enhancement" in WICKET-2846 is of very minor value to only a very small
> subset of cases that should rarely be used since they would not be
> considered best practices. And, if it were reverted, there would be fairly
> easy ways to get the same functionality without this change.
>
> So, I leave it to the community to vote on this. Because I feel neither +1
> or -1 on this issue, my (binding) vote will go to whatever non-binding
> majority wins. So, in this case, *every vote counts* - even if your vote is
> typically non-binding.
>
> As one last comment, please: we don't need any more long-running
> discussions or diatribes on this. We already know this issue intimately.
> Please simply vote, and if you must, provide a simple reason why you're
> voting the way you are.
>
> [ ] +1 - revert WICKET-2846 in the next release (in other words, get rid
> of the InheritableThreadLocal)
> [ ] -1 - leave everything exactly as it is (in other words, keep the
> InheritableThreadLocal)
>
> [*] - https://issues.apache.org/jira/browse/WICKET-2846
>
> Best regards,
>
> --
> Jeremy Thomerson
> http://www.wickettraining.com
>
>
[+1] - revert, binding on behalf of the non-binding votes
I didn't mention the voting period - but it was 72 hours, as usual. The
vote is overwhelmingly +1, revert. So, I will do this now.
Thanks everyone. InheritableThreadLocal will be gone in 1.4.10.
--
Jeremy Thomerson
http://www.wickettraining.com
Re: [vote] Revert WICKET-2846
Posted by Igor Vaynberg <ig...@gmail.com>.
+0
-igor
On Sat, May 22, 2010 at 12:22 AM, Jeremy Thomerson
<je...@wickettraining.com> wrote:
> As you may have noticed over the past couple of days (ha), there has been
> quite a bit of discussion over what seemed at the time like a very trivial
> change in WICKET-2846 [*]. The end result is that it does not break any
> existing applications that don't already have a bug. However, if the
> application is already faulty in the way it uses threads (perhaps even due
> to a bug within Java related to cleaning up Java2D threads), then the change
> in WICKET-2846 can exacerbate the issue. On the flip side, the
> "enhancement" in WICKET-2846 is of very minor value to only a very small
> subset of cases that should rarely be used since they would not be
> considered best practices. And, if it were reverted, there would be fairly
> easy ways to get the same functionality without this change.
>
> So, I leave it to the community to vote on this. Because I feel neither +1
> or -1 on this issue, my (binding) vote will go to whatever non-binding
> majority wins. So, in this case, *every vote counts* - even if your vote is
> typically non-binding.
>
> As one last comment, please: we don't need any more long-running discussions
> or diatribes on this. We already know this issue intimately. Please simply
> vote, and if you must, provide a simple reason why you're voting the way you
> are.
>
> [ ] +1 - revert WICKET-2846 in the next release (in other words, get rid of
> the InheritableThreadLocal)
> [ ] -1 - leave everything exactly as it is (in other words, keep the
> InheritableThreadLocal)
>
> [*] - https://issues.apache.org/jira/browse/WICKET-2846
>
> Best regards,
>
> --
> Jeremy Thomerson
> http://www.wickettraining.com
>
Re: [vote] Revert WICKET-2846
Posted by Francis De Brabandere <fr...@gmail.com>.
+1 - revert
On Sat, May 22, 2010 at 12:23 PM, Gwyn Evans <gw...@gmail.com> wrote:
>>
>> [X ] +1 - revert WICKET-2846 in the next release (in other words, get rid
>> of
>> the InheritableThreadLocal)
>>
>
> While there's a chance of not being able to avoid the thread problem due to
> it being in an 3rd-party code AND there's a workaround for what 2846 is
> doing, then it doesn't seem to be a good idea to include it.
>
> /Gwyn
>
--
http://www.somatik.be
Microsoft gives you windows, Linux gives you the whole house.
Re: [vote] Revert WICKET-2846
Posted by Gwyn Evans <gw...@gmail.com>.
>
> [X ] +1 - revert WICKET-2846 in the next release (in other words, get rid
> of
> the InheritableThreadLocal)
>
While there's a chance of not being able to avoid the thread problem due to
it being in an 3rd-party code AND there's a workaround for what 2846 is
doing, then it doesn't seem to be a good idea to include it.
/Gwyn
Re: [vote] Revert WICKET-2846
Posted by Johan Compagner <jc...@gmail.com>.
+1 revert it, i dont see a big pro in having it anyway
On Sat, May 22, 2010 at 09:22, Jeremy Thomerson
<je...@wickettraining.com>wrote:
> As you may have noticed over the past couple of days (ha), there has been
> quite a bit of discussion over what seemed at the time like a very trivial
> change in WICKET-2846 [*]. The end result is that it does not break any
> existing applications that don't already have a bug. However, if the
> application is already faulty in the way it uses threads (perhaps even due
> to a bug within Java related to cleaning up Java2D threads), then the
> change
> in WICKET-2846 can exacerbate the issue. On the flip side, the
> "enhancement" in WICKET-2846 is of very minor value to only a very small
> subset of cases that should rarely be used since they would not be
> considered best practices. And, if it were reverted, there would be fairly
> easy ways to get the same functionality without this change.
>
> So, I leave it to the community to vote on this. Because I feel neither +1
> or -1 on this issue, my (binding) vote will go to whatever non-binding
> majority wins. So, in this case, *every vote counts* - even if your vote
> is
> typically non-binding.
>
> As one last comment, please: we don't need any more long-running
> discussions
> or diatribes on this. We already know this issue intimately. Please
> simply
> vote, and if you must, provide a simple reason why you're voting the way
> you
> are.
>
> [ ] +1 - revert WICKET-2846 in the next release (in other words, get rid
> of
> the InheritableThreadLocal)
> [ ] -1 - leave everything exactly as it is (in other words, keep the
> InheritableThreadLocal)
>
> [*] - https://issues.apache.org/jira/browse/WICKET-2846
>
> Best regards,
>
> --
> Jeremy Thomerson
> http://www.wickettraining.com
>
Re: [vote] Revert WICKET-2846
Posted by James Carman <ja...@carmanconsulting.com>.
It has been shown many times why this change was a bad idea.
Nevermind the theoretical nature of the initial objections, which have
also been shown to be true concerns (see
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6489540). It has
been shown that this change doesn't really solve anything anyway. In
fact, it encourages a programming practice (starting short-lived,
one-off threads) that most feel is not a good idea and doesn't scale.
On Mon, May 24, 2010 at 4:40 PM, Alex Objelean <al...@gmail.com> wrote:
>
> My vote is -1. Unless we have a good way to prove the problem, not only
> theoretical presumption, we shouldn't revert it in next release.
> --
> View this message in context: http://apache-wicket.1842946.n4.nabble.com/vote-Revert-WICKET-2846-tp2226987p2229151.html
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
Re: [vote] Revert WICKET-2846
Posted by Alex Objelean <al...@gmail.com>.
My vote is -1. Unless we have a good way to prove the problem, not only
theoretical presumption, we shouldn't revert it in next release.
--
View this message in context: http://apache-wicket.1842946.n4.nabble.com/vote-Revert-WICKET-2846-tp2226987p2229151.html
Sent from the Wicket - Dev mailing list archive at Nabble.com.
Re: [vote] Revert WICKET-2846
Posted by James Carman <ja...@carmanconsulting.com>.
Or "draw" something yourself if you want to skip the dependency
On Tue, May 25, 2010 at 9:38 AM, James Carman
<ja...@carmanconsulting.com> wrote:
> Just try using something like JFreeChart
>
>
> On Tue, May 25, 2010 at 9:36 AM, Alex Objelean <al...@gmail.com> wrote:
>>
>> OK, then just give me whatever code you use which causes the memory leak...
>> --
>> View this message in context: http://apache-wicket.1842946.n4.nabble.com/vote-Revert-WICKET-2846-tp2226987p2230020.html
>> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>>
>
Re: [vote] Revert WICKET-2846
Posted by James Carman <ja...@carmanconsulting.com>.
Just try using something like JFreeChart
On Tue, May 25, 2010 at 9:36 AM, Alex Objelean <al...@gmail.com> wrote:
>
> OK, then just give me whatever code you use which causes the memory leak...
> --
> View this message in context: http://apache-wicket.1842946.n4.nabble.com/vote-Revert-WICKET-2846-tp2226987p2230020.html
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
Re: [vote] Revert WICKET-2846
Posted by Alex Objelean <al...@gmail.com>.
OK, then just give me whatever code you use which causes the memory leak...
--
View this message in context: http://apache-wicket.1842946.n4.nabble.com/vote-Revert-WICKET-2846-tp2226987p2230020.html
Sent from the Wicket - Dev mailing list archive at Nabble.com.
Re: [vote] Revert WICKET-2846
Posted by James Carman <ja...@carmanconsulting.com>.
You don't create the thread yourself. Java does!
On Tue, May 25, 2010 at 9:32 AM, Alex Objelean <al...@gmail.com> wrote:
>
> Hi Adriano again!
> I know you have no time for creating the quickstart. That is why I am trying
> to reproduce it. I want to make a quickstart to prove if the problem
> reported by you is true or false and will publish it when it is ready.
>
> I need some help from you. Could you give a simple example of how you create
> the thread in your application which handles the java2d and any other code
> you may find useful.
>
> Thanks!
> Alex
> --
> View this message in context: http://apache-wicket.1842946.n4.nabble.com/vote-Revert-WICKET-2846-tp2226987p2230013.html
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
Re: [vote] Revert WICKET-2846
Posted by Jeremy Thomerson <je...@wickettraining.com>.
On Wed, May 26, 2010 at 12:07 AM, Alex Objelean <al...@gmail.com>wrote:
>
> I'm glad we finally came to a conclusion. Do you think it would be useful
> to
> update the task description in JIRA? Even if it will remain 'WON"T FIX', I
> don't care, at least we have a proof and learn a little bit more from this
> experience.
>
> Thanks!
> Alex
> --
> View this message in context:
> http://apache-wicket.1842946.n4.nabble.com/vote-Revert-WICKET-2846-tp2226987p2231058.html
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
I'm not sure what you want added to the description. Just add a comment to
whatever effect you're looking for.
--
Jeremy Thomerson
http://www.wickettraining.com
Re: [vote] Revert WICKET-2846
Posted by Alex Objelean <al...@gmail.com>.
I'm glad we finally came to a conclusion. Do you think it would be useful to
update the task description in JIRA? Even if it will remain 'WON"T FIX', I
don't care, at least we have a proof and learn a little bit more from this
experience.
Thanks!
Alex
--
View this message in context: http://apache-wicket.1842946.n4.nabble.com/vote-Revert-WICKET-2846-tp2226987p2231058.html
Sent from the Wicket - Dev mailing list archive at Nabble.com.
Re: [vote] Revert WICKET-2846
Posted by Jeremy Thomerson <je...@wickettraining.com>.
On Tue, May 25, 2010 at 5:13 PM, Stefan Lindner <li...@visionet.de> wrote:
> -1
>
> Stefan
>
Voting has ended, it's already reverted.
--
Jeremy Thomerson
http://www.wickettraining.com
Re: [vote] Revert WICKET-2846
Posted by James Carman <ja...@carmanconsulting.com>.
Yeah, I saw that. I don't know if they manually clean the ITL's or
not, but they're definitely getting cleaned up. Perhaps trying it
with Jetty (unless someone knows that they also clean up after
themselves) might show that these references leak?
On Wed, May 26, 2010 at 1:18 AM, Martijn Dashorst
<ma...@gmail.com> wrote:
> The newest tomcat has improved permgen cleanup which they backported
> from tomcat 7 iiuc.
>
> Martijn
>
> On Wed, May 26, 2010 at 3:10 AM, James Carman
> <ja...@carmanconsulting.com> wrote:
>> I've done some playing with Tomcat undeploy and it does appear that
>> the ITL reference is cleared (perhaps Sun/Oracle/Sunacle should close
>> the "bug"?). However, this does not make the ITL implementation
>> valid, IMHO. It doesn't work (out of the box) for the preferred
>> method of executing asynchronous tasks, which is to use a thread pool.
>>
>> On Tue, May 25, 2010 at 6:38 PM, Alex Objelean <al...@gmail.com> wrote:
>>>
>>> Jeremy, I've tried with redeploy... same story. I'm not insisting on bringing
>>> the issue back. If is the willing of the majority, I'm ok. It is just to
>>> help me understand something that I'm not find that obvious.
>>>
>>> Alex
>>> --
>>> View this message in context: http://apache-wicket.1842946.n4.nabble.com/vote-Revert-WICKET-2846-tp2226987p2230845.html
>>> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>>>
>>
>
>
>
> --
> 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: [vote] Revert WICKET-2846
Posted by Martijn Dashorst <ma...@gmail.com>.
The newest tomcat has improved permgen cleanup which they backported
from tomcat 7 iiuc.
Martijn
On Wed, May 26, 2010 at 3:10 AM, James Carman
<ja...@carmanconsulting.com> wrote:
> I've done some playing with Tomcat undeploy and it does appear that
> the ITL reference is cleared (perhaps Sun/Oracle/Sunacle should close
> the "bug"?). However, this does not make the ITL implementation
> valid, IMHO. It doesn't work (out of the box) for the preferred
> method of executing asynchronous tasks, which is to use a thread pool.
>
> On Tue, May 25, 2010 at 6:38 PM, Alex Objelean <al...@gmail.com> wrote:
>>
>> Jeremy, I've tried with redeploy... same story. I'm not insisting on bringing
>> the issue back. If is the willing of the majority, I'm ok. It is just to
>> help me understand something that I'm not find that obvious.
>>
>> Alex
>> --
>> View this message in context: http://apache-wicket.1842946.n4.nabble.com/vote-Revert-WICKET-2846-tp2226987p2230845.html
>> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>>
>
--
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: [vote] Revert WICKET-2846
Posted by James Carman <ja...@carmanconsulting.com>.
I've done some playing with Tomcat undeploy and it does appear that
the ITL reference is cleared (perhaps Sun/Oracle/Sunacle should close
the "bug"?). However, this does not make the ITL implementation
valid, IMHO. It doesn't work (out of the box) for the preferred
method of executing asynchronous tasks, which is to use a thread pool.
On Tue, May 25, 2010 at 6:38 PM, Alex Objelean <al...@gmail.com> wrote:
>
> Jeremy, I've tried with redeploy... same story. I'm not insisting on bringing
> the issue back. If is the willing of the majority, I'm ok. It is just to
> help me understand something that I'm not find that obvious.
>
> Alex
> --
> View this message in context: http://apache-wicket.1842946.n4.nabble.com/vote-Revert-WICKET-2846-tp2226987p2230845.html
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
Re: [vote] Revert WICKET-2846
Posted by Alex Objelean <al...@gmail.com>.
Jeremy, I've tried with redeploy... same story. I'm not insisting on bringing
the issue back. If is the willing of the majority, I'm ok. It is just to
help me understand something that I'm not find that obvious.
Alex
--
View this message in context: http://apache-wicket.1842946.n4.nabble.com/vote-Revert-WICKET-2846-tp2226987p2230845.html
Sent from the Wicket - Dev mailing list archive at Nabble.com.
RE: [vote] Revert WICKET-2846
Posted by Stefan Lindner <li...@visionet.de>.
-1
Stefan
Re: [vote] Revert WICKET-2846
Posted by Jeremy Thomerson <je...@wickettraining.com>.
On Tue, May 25, 2010 at 4:57 PM, Alex Objelean <al...@gmail.com>wrote:
>
> James, thank you very much for such a detailed description. I appreciate
> your
> effort and I'm sure this experience helped me to learn a lot of new things.
> Nevertheless, I would like to clarify few things:
>
> Here is a definition of memory leak: "If a program holds a reference to a
> heap chunk that is not used during the rest of its life, it is considered a
> memory leak because the memory could have been freed and reused. GC won't
> reclaim it due to the reference being held by the program."
> I agree, your example proved that WicketApplication instance is used by
> InheritableThreadLocal (that is exactly what ITL does). Have you managed to
> find it even after the servlet container was stopped? Isn't it important to
> prove that the 'WicketApplication' instance outlives the servlet container
> and holds a reference to a heap chunk?
> To make it simpler to test, I have changed a little bit your quickstart,
> making the WicketApplication intance insanely big, by adding a very large
> object (50MB). My expectation was, in order to prove the memory leak, to
> see
> that the physical memory used by the JVM increases by 50MB after each
> restart, or at least steadily grow. You can check this by monitoring the
> 'Local' entry and the memory details from the right pane of the VisualVM
> window.
> But the actual behavior seems to be different: performing several times
> the same flow: "start the server->load the home page->stop the server"
> doesn't increase the heap memory usage. The memory usage comes back to
> normal after each restart.
>
> I do not want to draw the conclusion, because I could have a wrong
> understanding of what a 'memory leak' is and how to prove it correctly.
> That
> is why, I'm kindly asking you (or anybody else) to interpret my findings.
>
> Thank you!
> Regards,
> Alex Objelean
>
> PS: by the way, I'm using jdk 1.6.0_13-b03 if it does make any difference
>
>
Alex, this isn't leaking into sessions that are written to disk, so it would
not persist through a restart. The original reporter said specifically that
this happens on redeploys *without a restart*.
I've reverted this to a regular ThreadLocal for 1.4.10.
--
Jeremy Thomerson
http://www.wickettraining.com
Re: [vote] Revert WICKET-2846
Posted by tetsuo <ro...@gmail.com>.
It seems that this memory leak happens only once. I guess the first deploy
(the one which will force the creation of the background Java2D thread) will
hang around forever, but subsequent redeploys will be correctly cleaned-up.
Also, what leaks is the classloader. Only static variables and class
definitions will be kept in memory. So, you will only get an
OutOfMemoryError (and it will be a PermGen error) if your container has too
little permgen memory allocated, your application is realy huge (in class
number), or you keep very large objects in static vars, since it must break
with only two instances of the application.
That said... there's nothing like a working (err, breaking) test case to
prove a point!
On Tue, May 25, 2010 at 6:57 PM, Alex Objelean <al...@gmail.com>wrote:
>
> James, thank you very much for such a detailed description. I appreciate
> your
> effort and I'm sure this experience helped me to learn a lot of new things.
> Nevertheless, I would like to clarify few things:
>
> Here is a definition of memory leak: "If a program holds a reference to a
> heap chunk that is not used during the rest of its life, it is considered a
> memory leak because the memory could have been freed and reused. GC won't
> reclaim it due to the reference being held by the program."
> I agree, your example proved that WicketApplication instance is used by
> InheritableThreadLocal (that is exactly what ITL does). Have you managed to
> find it even after the servlet container was stopped? Isn't it important to
> prove that the 'WicketApplication' instance outlives the servlet container
> and holds a reference to a heap chunk?
> To make it simpler to test, I have changed a little bit your quickstart,
> making the WicketApplication intance insanely big, by adding a very large
> object (50MB). My expectation was, in order to prove the memory leak, to
> see
> that the physical memory used by the JVM increases by 50MB after each
> restart, or at least steadily grow. You can check this by monitoring the
> 'Local' entry and the memory details from the right pane of the VisualVM
> window.
> But the actual behavior seems to be different: performing several times
> the same flow: "start the server->load the home page->stop the server"
> doesn't increase the heap memory usage. The memory usage comes back to
> normal after each restart.
>
> I do not want to draw the conclusion, because I could have a wrong
> understanding of what a 'memory leak' is and how to prove it correctly.
> That
> is why, I'm kindly asking you (or anybody else) to interpret my findings.
>
> Thank you!
> Regards,
> Alex Objelean
>
> PS: by the way, I'm using jdk 1.6.0_13-b03 if it does make any difference
>
> --
> View this message in context:
> http://apache-wicket.1842946.n4.nabble.com/vote-Revert-WICKET-2846-tp2226987p2230796.html
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
Re: [vote] Revert WICKET-2846
Posted by Alex Objelean <al...@gmail.com>.
James, thank you very much for such a detailed description. I appreciate your
effort and I'm sure this experience helped me to learn a lot of new things.
Nevertheless, I would like to clarify few things:
Here is a definition of memory leak: "If a program holds a reference to a
heap chunk that is not used during the rest of its life, it is considered a
memory leak because the memory could have been freed and reused. GC won't
reclaim it due to the reference being held by the program."
I agree, your example proved that WicketApplication instance is used by
InheritableThreadLocal (that is exactly what ITL does). Have you managed to
find it even after the servlet container was stopped? Isn't it important to
prove that the 'WicketApplication' instance outlives the servlet container
and holds a reference to a heap chunk?
To make it simpler to test, I have changed a little bit your quickstart,
making the WicketApplication intance insanely big, by adding a very large
object (50MB). My expectation was, in order to prove the memory leak, to see
that the physical memory used by the JVM increases by 50MB after each
restart, or at least steadily grow. You can check this by monitoring the
'Local' entry and the memory details from the right pane of the VisualVM
window.
But the actual behavior seems to be different: performing several times
the same flow: "start the server->load the home page->stop the server"
doesn't increase the heap memory usage. The memory usage comes back to
normal after each restart.
I do not want to draw the conclusion, because I could have a wrong
understanding of what a 'memory leak' is and how to prove it correctly. That
is why, I'm kindly asking you (or anybody else) to interpret my findings.
Thank you!
Regards,
Alex Objelean
PS: by the way, I'm using jdk 1.6.0_13-b03 if it does make any difference
--
View this message in context: http://apache-wicket.1842946.n4.nabble.com/vote-Revert-WICKET-2846-tp2226987p2230796.html
Sent from the Wicket - Dev mailing list archive at Nabble.com.
Re: [vote] Revert WICKET-2846
Posted by James Carman <ja...@carmanconsulting.com>.
I ran this page and then fired up jvisualvm and took a "heap dump."
Go to the "Classes" view. Find the java.lang.Thread class. Click on
it and it will bring up a list of instances. Go through them until
you find one with the name "Java2D Disposer" (#13 for me). Then, look
for the field called "inheritableThreadLocals" and look in its "table"
field. In there, you'll find a reference to the WicketApplication
object. There's your leak! The Java2D Disposer thread runs for the
duration of the VM and won't let go of that sucker.
On Tue, May 25, 2010 at 10:15 AM, James Carman
<ja...@carmanconsulting.com> wrote:
> Here's an example page that you can plug into a quickstart to show the bug:
>
> public class HomePage extends WebPage
> {
> private static final long serialVersionUID = 1L;
>
> static
> {
> System.out.println("I'm being initialized within the context
> of thread " + Thread.currentThread().getName() + ".");
> }
>
> public HomePage()
> {
> add(new Image("image", new MyImageResource()));
> }
>
> private class MyImageResource extends DynamicImageResource
> {
> private MyImageResource()
> {
> super("png");
> }
>
> @Override
> protected byte[] getImageData()
> {
> final BufferedImage bufferedImage = new BufferedImage(100,
> 100, BufferedImage.TYPE_4BYTE_ABGR);
> Graphics2D g2d = (Graphics2D) bufferedImage.getGraphics();
> g2d.setPaint(Color.white);
> g2d.fillRect(0, 0, 100, 100);
> g2d.setPaint(Color.red);
> g2d.drawOval(45, 45, 10, 10);
> final ByteArrayOutputStream bout = new ByteArrayOutputStream();
> try
> {
> ImageIO.write(bufferedImage, "png", bout);
> bout.close();
> }
> catch (IOException e)
> {
> throw new WicketRuntimeException("Unable to render image.", e);
> }
> return bout.toByteArray();
> }
> }
> }
>
> On Tue, May 25, 2010 at 10:08 AM, Alex Objelean <al...@gmail.com> wrote:
>>
>> Thanks!
>> It is enough for me. I'll do the rest from here..
>>
>> Alex
>>
>>
>> --
>> View this message in context: http://apache-wicket.1842946.n4.nabble.com/vote-Revert-WICKET-2846-tp2226987p2230086.html
>> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>>
>
Re: [vote] Revert WICKET-2846
Posted by James Carman <ja...@carmanconsulting.com>.
Here's an example page that you can plug into a quickstart to show the bug:
public class HomePage extends WebPage
{
private static final long serialVersionUID = 1L;
static
{
System.out.println("I'm being initialized within the context
of thread " + Thread.currentThread().getName() + ".");
}
public HomePage()
{
add(new Image("image", new MyImageResource()));
}
private class MyImageResource extends DynamicImageResource
{
private MyImageResource()
{
super("png");
}
@Override
protected byte[] getImageData()
{
final BufferedImage bufferedImage = new BufferedImage(100,
100, BufferedImage.TYPE_4BYTE_ABGR);
Graphics2D g2d = (Graphics2D) bufferedImage.getGraphics();
g2d.setPaint(Color.white);
g2d.fillRect(0, 0, 100, 100);
g2d.setPaint(Color.red);
g2d.drawOval(45, 45, 10, 10);
final ByteArrayOutputStream bout = new ByteArrayOutputStream();
try
{
ImageIO.write(bufferedImage, "png", bout);
bout.close();
}
catch (IOException e)
{
throw new WicketRuntimeException("Unable to render image.", e);
}
return bout.toByteArray();
}
}
}
On Tue, May 25, 2010 at 10:08 AM, Alex Objelean <al...@gmail.com> wrote:
>
> Thanks!
> It is enough for me. I'll do the rest from here..
>
> Alex
>
>
> --
> View this message in context: http://apache-wicket.1842946.n4.nabble.com/vote-Revert-WICKET-2846-tp2226987p2230086.html
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
Re: [vote] Revert WICKET-2846
Posted by Alex Objelean <al...@gmail.com>.
Thanks!
It is enough for me. I'll do the rest from here..
Alex
--
View this message in context: http://apache-wicket.1842946.n4.nabble.com/vote-Revert-WICKET-2846-tp2226987p2230086.html
Sent from the Wicket - Dev mailing list archive at Nabble.com.
Re: [vote] Revert WICKET-2846
Posted by Adriano dos Santos Fernandes <ad...@gmail.com>.
On 25/05/2010 10:47, Alex Objelean wrote:
> That is exactly what I am trying to do. To make a simple example that cause
> the leak and monitor it... By quickstart I mean the simplest project which
> helps to cause the leak.
>
package mm.util.app;
import java.awt.GraphicsEnvironment;
public class Teste
{
public static void main(String[] args) throws Exception
{
System.out.println(Thread.getAllStackTraces().size());
GraphicsEnvironment.getLocalGraphicsEnvironment().getAvailableFontFamilyNames();
System.out.println(Thread.getAllStackTraces().size());
}
}
Output:
5
6
Do I also need to prove that ITL gos to new threads or no?
Adriano
Re: [vote] Revert WICKET-2846
Posted by Alex Objelean <al...@gmail.com>.
That is exactly what I am trying to do. To make a simple example that cause
the leak and monitor it... By quickstart I mean the simplest project which
helps to cause the leak.
--
View this message in context: http://apache-wicket.1842946.n4.nabble.com/vote-Revert-WICKET-2846-tp2226987p2230043.html
Sent from the Wicket - Dev mailing list archive at Nabble.com.
Re: [vote] Revert WICKET-2846
Posted by Adriano dos Santos Fernandes <ad...@gmail.com>.
Sorry, but notion of "Wicket quickstart" for this issue has no sense.
Look, I shown you a opened bug report. I also shown how I force the
java2d thread creation. Note, I create no threads myself, and AFAIR,
even some (do not remember what) Wicket code triggered the java2d thread
creation.
As a mater of java2d thread creation, it will got any inheritable thread
local. That's not theory, it's fact!
So why I said it's not about quickstart, you must them open something
(jvisualvm?) and monitor threads. Or you may also play with classloader
and a not-small application and verify a leak.
Adriano
On 25/05/2010 10:32, Alex Objelean wrote:
> Hi Adriano again!
> I know you have no time for creating the quickstart. That is why I am trying
> to reproduce it. I want to make a quickstart to prove if the problem
> reported by you is true or false and will publish it when it is ready.
>
> I need some help from you. Could you give a simple example of how you create
> the thread in your application which handles the java2d and any other code
> you may find useful.
>
> Thanks!
> Alex
>
Re: [vote] Revert WICKET-2846
Posted by Alex Objelean <al...@gmail.com>.
Hi Adriano again!
I know you have no time for creating the quickstart. That is why I am trying
to reproduce it. I want to make a quickstart to prove if the problem
reported by you is true or false and will publish it when it is ready.
I need some help from you. Could you give a simple example of how you create
the thread in your application which handles the java2d and any other code
you may find useful.
Thanks!
Alex
--
View this message in context: http://apache-wicket.1842946.n4.nabble.com/vote-Revert-WICKET-2846-tp2226987p2230013.html
Sent from the Wicket - Dev mailing list archive at Nabble.com.
Re: [vote] Revert WICKET-2846
Posted by Alex Objelean <al...@gmail.com>.
Hi Adriano!
Since you are the person who is started the Anti-2847 movement :), and since
the new release 1.4.9 is out with 2846 included, I think that there is a
good chance for you and for all those who voted +1 for reverting it to prove
the problem.. I am still not convinced in validity of your arguments, but
I'll take my words back if you can prove it.
Thanks!
Alex
--
View this message in context: http://apache-wicket.1842946.n4.nabble.com/vote-Revert-WICKET-2846-tp2226987p2229141.html
Sent from the Wicket - Dev mailing list archive at Nabble.com.
Re: [vote] Revert WICKET-2846
Posted by Adriano dos Santos Fernandes <ad...@gmail.com>.
+1 - revert
...
On 22-05-2010 19:34, Jeremy Thomerson wrote:
>>
> Please see the other threads for more details. In short, there is a Java
> bug that if you are manipulating images, it may not clean up Java2D threads
> correctly. There is a known workaround - initializing a cleanup thread
> yourself. But, if you aren't doing this, and you start threads that
> manipulate images, and you redeploy without a restart, then you leak those
> threads.
It's not threads that are leaked. It's the web application classloader.
Adriano
Re: [vote] Revert WICKET-2846
Posted by Jeremy Thomerson <je...@wickettraining.com>.
On Sat, May 22, 2010 at 2:32 PM, Douglas Ferguson <
douglas@douglasferguson.us> wrote:
> Under what circumstances does this become a problem?
>
> I'm just curious about more details on this comment:
>
> "The end result is that it does not break any existing applications that
> don't already have a bug"
>
>
Please see the other threads for more details. In short, there is a Java
bug that if you are manipulating images, it may not clean up Java2D threads
correctly. There is a known workaround - initializing a cleanup thread
yourself. But, if you aren't doing this, and you start threads that
manipulate images, and you redeploy without a restart, then you leak those
threads. (You can see this is a pretty rare case). Now, never before would
we have put anything that consumes memory into your threads automatically.
But, with the InheritableThreadLocal, the Application will
get propagated to these threads. So, if you already had the existing bug of
leaking threads, now we've added something that will also leak your memory
with those threads. So, "it does not break any existing applications that
don't already have a bug" - the thread leaking bug. But, it can make the
existing bug worse (or, you could see it as a nice thing - it brings it to
your attention if you weren't aware of it :)
--
Jeremy Thomerson
http://www.wickettraining.com
Re: [vote] Revert WICKET-2846
Posted by Douglas Ferguson <do...@douglasferguson.us>.
Under what circumstances does this become a problem?
I'm just curious about more details on this comment:
"The end result is that it does not break any existing applications that don't already have a bug"
D/
On May 22, 2010, at 2:22 AM, Jeremy Thomerson wrote:
> As you may have noticed over the past couple of days (ha), there has been
> quite a bit of discussion over what seemed at the time like a very trivial
> change in WICKET-2846 [*]. The end result is that it does not break any
> existing applications that don't already have a bug. However, if the
> application is already faulty in the way it uses threads (perhaps even due
> to a bug within Java related to cleaning up Java2D threads), then the change
> in WICKET-2846 can exacerbate the issue. On the flip side, the
> "enhancement" in WICKET-2846 is of very minor value to only a very small
> subset of cases that should rarely be used since they would not be
> considered best practices. And, if it were reverted, there would be fairly
> easy ways to get the same functionality without this change.
>
> So, I leave it to the community to vote on this. Because I feel neither +1
> or -1 on this issue, my (binding) vote will go to whatever non-binding
> majority wins. So, in this case, *every vote counts* - even if your vote is
> typically non-binding.
>
> As one last comment, please: we don't need any more long-running discussions
> or diatribes on this. We already know this issue intimately. Please simply
> vote, and if you must, provide a simple reason why you're voting the way you
> are.
>
> [ ] +1 - revert WICKET-2846 in the next release (in other words, get rid of
> the InheritableThreadLocal)
> [ ] -1 - leave everything exactly as it is (in other words, keep the
> InheritableThreadLocal)
>
> [*] - https://issues.apache.org/jira/browse/WICKET-2846
>
> Best regards,
>
> --
> Jeremy Thomerson
> http://www.wickettraining.com
Re: [vote] Revert WICKET-2846
Posted by Sven Meier <sv...@meiers.net>.
My vote counts? Great ;)
+1
On 05/22/2010 09:22 AM, Jeremy Thomerson wrote:
> As you may have noticed over the past couple of days (ha), there has been
> quite a bit of discussion over what seemed at the time like a very trivial
> change in WICKET-2846 [*]. The end result is that it does not break any
> existing applications that don't already have a bug. However, if the
> application is already faulty in the way it uses threads (perhaps even due
> to a bug within Java related to cleaning up Java2D threads), then the change
> in WICKET-2846 can exacerbate the issue. On the flip side, the
> "enhancement" in WICKET-2846 is of very minor value to only a very small
> subset of cases that should rarely be used since they would not be
> considered best practices. And, if it were reverted, there would be fairly
> easy ways to get the same functionality without this change.
>
> So, I leave it to the community to vote on this. Because I feel neither +1
> or -1 on this issue, my (binding) vote will go to whatever non-binding
> majority wins. So, in this case, *every vote counts* - even if your vote is
> typically non-binding.
>
> As one last comment, please: we don't need any more long-running discussions
> or diatribes on this. We already know this issue intimately. Please simply
> vote, and if you must, provide a simple reason why you're voting the way you
> are.
>
> [ ] +1 - revert WICKET-2846 in the next release (in other words, get rid of
> the InheritableThreadLocal)
> [ ] -1 - leave everything exactly as it is (in other words, keep the
> InheritableThreadLocal)
>
> [*] - https://issues.apache.org/jira/browse/WICKET-2846
>
> Best regards,
>
> --
> Jeremy Thomerson
> http://www.wickettraining.com
>
>