You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@ant.apache.org by Gan Dong <ho...@gmail.com> on 2015/01/15 07:08:03 UTC
org.apache.tools.ant.property.LocalProperties cannot be extended
Hi experts,
Recently, I came across a problem in Ant and the solution seems to
write a custom LocalProperties implementation to fix the issue. But I
found that this class has a sole privet constructor which prevents me
from sub-classing it:
55 /**
56 * Construct a new LocalProperties object.
57 */
58 private LocalProperties() {
59 }
I'd like to know, is such design on purpose? What's the consideration
in making it private?
I know Ant provided a mechanism as following which seems to have the
ability to register user-defined LocalProperties class:
LocalProperties l = (LocalProperties) project.getReference(
MagicNames.REFID_LOCAL_PROPERTIES);
(above line is from line 39 of ant source code
https://git-wip-us.apache.org/repos/asf?p=ant.git;a=blob;f=src/main/org/apache/tools/ant/property/LocalProperties.java;h=fb2fa9c0a80a4cadf0fbc3098eedef16fd710aa9;hb=1c927b15af84cfce315a0ef6f4db60c7d47c2c50,
also seen from many other tasks )
But actually it doesn't work as LocalProperties cannot be extended,
thus above assigning would cause class cast issue. So I'm confused, if
this is not supposed to be extended, then what's the point of having
MagicNames.REFID_LOCAL_PROPERTIES?
Some additional context on why I want to extend the class. Due to this
ant bug: https://issues.apache.org/bugzilla/show_bug.cgi?id=55074, our
framework, which calls ant, occasionally throws
ConcurrentModificationException when parallel is used. To fix the
thread-safe issue in LocalPropertyStack on our side, I'd like to
create an alternative one by overriding following method in
LocalProperties:
61 /**
62 * Get the initial value.
63 * @return a new localproperties stack.
64 */
65 protected synchronized LocalPropertyStack initialValue() {
66 return new LocalPropertyStack();
67 }
Any input would be appreciated.
Thanks,
Gan
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@ant.apache.org
For additional commands, e-mail: user-help@ant.apache.org
Re: org.apache.tools.ant.property.LocalProperties cannot be extended
Posted by Gan Dong <ho...@gmail.com>.
Thanks, Stefan. I've updated the bugzilla issue with my findings.
Gan
On Mon, Jan 26, 2015 at 5:23 PM, Stefan Bodewig <bo...@apache.org> wrote:
>
> On 2015-01-26, Gan Dong wrote:
>
>> I investigated into this problem in our user's environment, and found
>> that it was using Ant-contrib's "forget" task
>> (http://ant-contrib.sourceforge.net/tasks/tasks/forget.html).
>
> Thanks Gan, that's unfortunate. Could you please update the Bugzilla
> issue with your findings? I'll try to validate what you've found later
> this week.
>
> My first reaction would be to say "get <forget> fixed", but I don't
> think you'll find anybody cutting a new AntContrib release[1] - you can
> try, of course.
>
> OTOH Ant is not safe from third party tasks not playing to the rules, so
> we'll need to find a way to deal with it. I'd prefer to avoid
> synchronization. The exceptions you see are really just a symptom. If
> we start to accept that LocalPropertyStack sometimes leaks outside of
> "its" thread, this may mean even more trouble spots. I'm not quite sure
> how to deal with that.
>
> Stefan
>
> [1] Technically I'm an AntContrib committer myself and can certainly fix
> the code. TBH I haven't done anything for the project for years and I
> don't think anybody else has. In particular I think I'd lack the power
> to publish a new release at SF (never tried).
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: user-unsubscribe@ant.apache.org
> For additional commands, e-mail: user-help@ant.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@ant.apache.org
For additional commands, e-mail: user-help@ant.apache.org
Re: org.apache.tools.ant.property.LocalProperties cannot be extended
Posted by Stefan Bodewig <bo...@apache.org>.
On 2015-01-26, Gan Dong wrote:
> I investigated into this problem in our user's environment, and found
> that it was using Ant-contrib's "forget" task
> (http://ant-contrib.sourceforge.net/tasks/tasks/forget.html).
Thanks Gan, that's unfortunate. Could you please update the Bugzilla
issue with your findings? I'll try to validate what you've found later
this week.
My first reaction would be to say "get <forget> fixed", but I don't
think you'll find anybody cutting a new AntContrib release[1] - you can
try, of course.
OTOH Ant is not safe from third party tasks not playing to the rules, so
we'll need to find a way to deal with it. I'd prefer to avoid
synchronization. The exceptions you see are really just a symptom. If
we start to accept that LocalPropertyStack sometimes leaks outside of
"its" thread, this may mean even more trouble spots. I'm not quite sure
how to deal with that.
Stefan
[1] Technically I'm an AntContrib committer myself and can certainly fix
the code. TBH I haven't done anything for the project for years and I
don't think anybody else has. In particular I think I'd lack the power
to publish a new release at SF (never tried).
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@ant.apache.org
For additional commands, e-mail: user-help@ant.apache.org
Re: org.apache.tools.ant.property.LocalProperties cannot be extended
Posted by Gan Dong <ho...@gmail.com>.
Hi Stefan,
Thanks for the clarification.
I investigated into this problem in our user's environment, and found
that it was using Ant-contrib's "forget" task
(http://ant-contrib.sourceforge.net/tasks/tasks/forget.html). The task
simply created a child thread of main ant thread, and using this new
thread to run nested ant tasks in background. I think this might be
the cause. Below are my observation, can you please review and see if
it makes sense.
While "parallel" task provided by ant always makes a new copy of
LocalPropertyStack for the child thread when it enters the run()
method, the ant-contrib "forget" task does not do that. It simply
creates a new thread and perform the task using the thread.
==== from Parallel.java ====
public void run() {
try {
LocalProperties.get(getProject()).copy();
....
==== from ForgetTask.java ====
public void execute()
{
Thread t = new Thread(this);
t.setDaemon(daemon);
t.start();
}
public void run()
{
super.execute();
}
On the other hand, LocalProperties is an InheritableThreadLocal, which
means the parent thread and child threads are sharing the same
LocalPropertyTask. In this case, the child thread that runs nested
tasks is the child of the main Ant thread, thus it actually has the
same LocalThreadStack as main ant thread. So this leads to the
concurrent issue between main ant thread and child forget tasks
thread, or between different child threads of "forget" tasks.
I also wrote below reproducer to verify above thought.
<target name="run-forget">
<forget>
<antcall target="job1" />
</forget>
<forget>
<antcall target="job2" >
<param name="dummy" value="foo" />
</antcall>
</forget>
</target>
<target name="job1">
<property name="dummy" value="job1" />
<echo message="JOB1: dummy=${dummy}" />
<sleep seconds="1" />
</target>
<target name="job2">
<property name="dummy" value="job2" />
<echo message="JOB2: dummy=${dummy}" />
</target>
When I call above "run-forget" task in a couple times, I get various
of exceptions quite often.
But when I use <parallel> to simulate above, I don't get any problem.
So it seems that "forget" usage was the cause. I guess this task was
wrote before LocalProperties was introduced, and hadn't been updated
since then.
Anyway, with above assumption, I'm going to fix the problem in our
framework by overriding the ant-contrib "forget" task with ours, which
does copy the LocalPropertyStack for new threads. Hopefully, it can
resolve the issue in our environment.
However, I still have a question. Do you think there would be a good
way from Ant side to prevent such concurrent issue happens again. I
mean, we can't stop users from writing custom tasks that can run tasks
in a multithreading way. But can we improve LocalProperties and make
it immune from such usage.
Thanks,
Gan
On Tue, Jan 20, 2015 at 11:11 PM, Stefan Bodewig <bo...@apache.org> wrote:
> On 2015-01-15, Gan Dong wrote:
>
>> Recently, I came across a problem in Ant and the solution seems to
>> write a custom LocalProperties implementation to fix the issue. But I
>> found that this class has a sole privet constructor which prevents me
>> from sub-classing it:
>
>> 55 /**
>> 56 * Construct a new LocalProperties object.
>> 57 */
>> 58 private LocalProperties() {
>> 59 }
>
>> I'd like to know, is such design on purpose? What's the consideration
>> in making it private?
>
> Basically because we never felt this is an extension point we want to
> support. The class is not meant to be subclassed.
>
>> I know Ant provided a mechanism as following which seems to have the
>> ability to register user-defined LocalProperties class:
>
>> LocalProperties l = (LocalProperties) project.getReference(
>> MagicNames.REFID_LOCAL_PROPERTIES);
>
> Ah, no. There should only be a single LocalProperties stack for a
> project and the code above is getting access to that. Like a global
> variable but hidden behind some smoke and mirrors.
>
>> Some additional context on why I want to extend the class. Due to this
>> ant bug: https://issues.apache.org/bugzilla/show_bug.cgi?id=55074, our
>> framework, which calls ant, occasionally throws
>> ConcurrentModificationException when parallel is used.
>
> I've been meaning to look into this, but failed to find the time to do
> so so far. LocalPropertyStack is supposed to be used only by a single
> thread at a time, I'm surprised to see the
> ConcurrentModificationException. Rather than synchronizing the stack
> I'd like to figure out where it leaks outside of its Thread and fix
> that.
>
> Cheers
>
> Stefan
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: user-unsubscribe@ant.apache.org
> For additional commands, e-mail: user-help@ant.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@ant.apache.org
For additional commands, e-mail: user-help@ant.apache.org
Re: org.apache.tools.ant.property.LocalProperties cannot be extended
Posted by Stefan Bodewig <bo...@apache.org>.
On 2015-01-15, Gan Dong wrote:
> Recently, I came across a problem in Ant and the solution seems to
> write a custom LocalProperties implementation to fix the issue. But I
> found that this class has a sole privet constructor which prevents me
> from sub-classing it:
> 55 /**
> 56 * Construct a new LocalProperties object.
> 57 */
> 58 private LocalProperties() {
> 59 }
> I'd like to know, is such design on purpose? What's the consideration
> in making it private?
Basically because we never felt this is an extension point we want to
support. The class is not meant to be subclassed.
> I know Ant provided a mechanism as following which seems to have the
> ability to register user-defined LocalProperties class:
> LocalProperties l = (LocalProperties) project.getReference(
> MagicNames.REFID_LOCAL_PROPERTIES);
Ah, no. There should only be a single LocalProperties stack for a
project and the code above is getting access to that. Like a global
variable but hidden behind some smoke and mirrors.
> Some additional context on why I want to extend the class. Due to this
> ant bug: https://issues.apache.org/bugzilla/show_bug.cgi?id=55074, our
> framework, which calls ant, occasionally throws
> ConcurrentModificationException when parallel is used.
I've been meaning to look into this, but failed to find the time to do
so so far. LocalPropertyStack is supposed to be used only by a single
thread at a time, I'm surprised to see the
ConcurrentModificationException. Rather than synchronizing the stack
I'd like to figure out where it leaks outside of its Thread and fix
that.
Cheers
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@ant.apache.org
For additional commands, e-mail: user-help@ant.apache.org