You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@groovy.apache.org by Jacopo Cappellato <ja...@gmail.com> on 2015/05/02 12:51:47 UTC
Thread safety and OptionBuilder
Hi all,
in the GroovyMain class there is the following code:
/**
* Build the options parser. Has to be synchronized because of the way Options are constructed.
*
* @return an options parser.
*/
private static synchronized Options buildOptions() {
Options options = new Options();
options.addOption(OptionBuilder.hasArg()....
...
The JavaDoc comment says:
"Has to be synchronized because of the way Options are constructed."
but unfortunately this is not enough to make the Groovy code thread safe.
In fact the current version of Commons CLI used by Groovy is 1.2, which is the latest stable release, and in this version the OptionBuilder class is clearly not meant to be used by more than one thread (there are several static fields modified by the builder methods); the issue has been reported a few times (e.g. https://issues.apache.org/jira/browse/CLI-209) and has been fixed by deprecating the class in favor of a new Option.builder() method:
https://commons.apache.org/proper/commons-cli/apidocs/org/apache/commons/cli/OptionBuilder.html
Unfortunately I don't know if Commons-CLI 1.3 will be released soon (1.2. was released 2 years ago). And the OptionBuilder class is used in a few classes in Groovy.
My question is: is this a concern/known issue for Groovy? what is the best way to address the issue? Should we get in touch with the Commons CLI community to know more about their plans or should me look for a different approach? With this information I will try to help to move in the direction you feel is the best.
Regards,
Jacopo
Re: Thread safety and OptionBuilder
Posted by Paul King <pa...@asert.com.au>.
Nice! Did you attempt to remove GroovyPosixParser and GroovyInternalPosixParser or just let them co-exist with 1.3?
Cheers, Paul.
On 6/05/2015 3:22 AM, Pascal Schumacher wrote:
> I replaced the commons-cli 1.2 dependency of Groovy with the 1.3 rc and ran some checks:
>
> - Build passes
> - Tests pass
> - Quick manual check using groovysh successful
>
> Am 03.05.2015 um 20:43 schrieb Pascal Schumacher:
>> Am 03.05.2015 um 13:03 schrieb Benedikt Ritter:
>>> Good point. I could try to do the pre release clean up later today and get an RC out this week.
>>
>> Thanks for creating the RC:
>>
>> https://mail-archives.apache.org/mod_mbox/commons-dev/201505.mbox/%3CCAB917RKab4oO_OMa_FcMuEegZwKA%2BNQNNOjjMnYE9QDGtNHXdg%40mail.gmail.com%3E
>>
>> :)
>
>
---
This email has been checked for viruses by Avast antivirus software.
http://www.avast.com
Re: Thread safety and OptionBuilder
Posted by Jacopo Cappellato <ja...@gmail.com>.
I have submitted a pull request (https://github.com/apache/incubator-groovy/pull/11 ) to convert the code that is using the deprecated OptionBuilder class.
Please let me know if this is the appropriate/preferred way to contribute or if you would like me to create a Jira ticket etc...
Thanks,
Jacopo
On May 8, 2015, at 8:33 AM, Jacopo Cappellato <ja...@gmail.com> wrote:
> Just a quick update on this topic: I have a local feature branch with a complete conversion from OptionBuilder to Option.builder(...) api.
> Everything seems to work fine so far.
> In the meantime the vote to release Commons CLI 1.3 is going on but so far only 2 PMC members cast their vote, so it is still uncertain if the vote will be successful.
>
> Regards,
>
> Jacopo
>
> On May 7, 2015, at 7:15 AM, Jacopo Cappellato <ja...@gmail.com> wrote:
>
>> Thank you Pascal! Much appreciated.
>>
>> Jacopo
>>
>> On May 6, 2015, at 7:51 PM, Pascal Schumacher <pa...@gmx.net> wrote:
>>
>>> Hi Jacopo,
>>>
>>> I pushed the change to my clone of the groovy repository on github. This is what I did:
>>>
>>> https://github.com/PascalSchumacher/incubator-groovy/commit/205a0b7188612b386038bcd01b4a1184e51d9be7
>>>
>>> - Pascal
>>>
>>> Am 06.05.2015 um 14:44 schrieb Jacopo Cappellato:
>>>> Hi Pascal,
>>>>
>>>> what is the easiest way to replace it in my dev box? I would like to test the same and try to migrate some of the code to the new API.
>>>>
>>>> Thanks,
>>>>
>>>> Jacopo
>>>>
>>>> On May 5, 2015, at 7:22 PM, Pascal Schumacher <pa...@gmx.net> wrote:
>>>>
>>>>> I replaced the commons-cli 1.2 dependency of Groovy with the 1.3 rc and ran some checks:
>>>>>
>>>>> - Build passes
>>>>> - Tests pass
>>>>> - Quick manual check using groovysh successful
>>>>>
>>>>> Am 03.05.2015 um 20:43 schrieb Pascal Schumacher:
>>>>>> Am 03.05.2015 um 13:03 schrieb Benedikt Ritter:
>>>>>>> Good point. I could try to do the pre release clean up later today and get an RC out this week.
>>>>>> Thanks for creating the RC:
>>>>>>
>>>>>> https://mail-archives.apache.org/mod_mbox/commons-dev/201505.mbox/%3CCAB917RKab4oO_OMa_FcMuEegZwKA%2BNQNNOjjMnYE9QDGtNHXdg%40mail.gmail.com%3E
>>>>>>
>>>>>> :)
>>>
>>
>
Re: Thread safety and OptionBuilder
Posted by Jacopo Cappellato <ja...@gmail.com>.
Just a quick update on this topic: I have a local feature branch with a complete conversion from OptionBuilder to Option.builder(...) api.
Everything seems to work fine so far.
In the meantime the vote to release Commons CLI 1.3 is going on but so far only 2 PMC members cast their vote, so it is still uncertain if the vote will be successful.
Regards,
Jacopo
On May 7, 2015, at 7:15 AM, Jacopo Cappellato <ja...@gmail.com> wrote:
> Thank you Pascal! Much appreciated.
>
> Jacopo
>
> On May 6, 2015, at 7:51 PM, Pascal Schumacher <pa...@gmx.net> wrote:
>
>> Hi Jacopo,
>>
>> I pushed the change to my clone of the groovy repository on github. This is what I did:
>>
>> https://github.com/PascalSchumacher/incubator-groovy/commit/205a0b7188612b386038bcd01b4a1184e51d9be7
>>
>> - Pascal
>>
>> Am 06.05.2015 um 14:44 schrieb Jacopo Cappellato:
>>> Hi Pascal,
>>>
>>> what is the easiest way to replace it in my dev box? I would like to test the same and try to migrate some of the code to the new API.
>>>
>>> Thanks,
>>>
>>> Jacopo
>>>
>>> On May 5, 2015, at 7:22 PM, Pascal Schumacher <pa...@gmx.net> wrote:
>>>
>>>> I replaced the commons-cli 1.2 dependency of Groovy with the 1.3 rc and ran some checks:
>>>>
>>>> - Build passes
>>>> - Tests pass
>>>> - Quick manual check using groovysh successful
>>>>
>>>> Am 03.05.2015 um 20:43 schrieb Pascal Schumacher:
>>>>> Am 03.05.2015 um 13:03 schrieb Benedikt Ritter:
>>>>>> Good point. I could try to do the pre release clean up later today and get an RC out this week.
>>>>> Thanks for creating the RC:
>>>>>
>>>>> https://mail-archives.apache.org/mod_mbox/commons-dev/201505.mbox/%3CCAB917RKab4oO_OMa_FcMuEegZwKA%2BNQNNOjjMnYE9QDGtNHXdg%40mail.gmail.com%3E
>>>>>
>>>>> :)
>>
>
Re: Thread safety and OptionBuilder
Posted by Jacopo Cappellato <ja...@gmail.com>.
Thank you Pascal! Much appreciated.
Jacopo
On May 6, 2015, at 7:51 PM, Pascal Schumacher <pa...@gmx.net> wrote:
> Hi Jacopo,
>
> I pushed the change to my clone of the groovy repository on github. This is what I did:
>
> https://github.com/PascalSchumacher/incubator-groovy/commit/205a0b7188612b386038bcd01b4a1184e51d9be7
>
> - Pascal
>
> Am 06.05.2015 um 14:44 schrieb Jacopo Cappellato:
>> Hi Pascal,
>>
>> what is the easiest way to replace it in my dev box? I would like to test the same and try to migrate some of the code to the new API.
>>
>> Thanks,
>>
>> Jacopo
>>
>> On May 5, 2015, at 7:22 PM, Pascal Schumacher <pa...@gmx.net> wrote:
>>
>>> I replaced the commons-cli 1.2 dependency of Groovy with the 1.3 rc and ran some checks:
>>>
>>> - Build passes
>>> - Tests pass
>>> - Quick manual check using groovysh successful
>>>
>>> Am 03.05.2015 um 20:43 schrieb Pascal Schumacher:
>>>> Am 03.05.2015 um 13:03 schrieb Benedikt Ritter:
>>>>> Good point. I could try to do the pre release clean up later today and get an RC out this week.
>>>> Thanks for creating the RC:
>>>>
>>>> https://mail-archives.apache.org/mod_mbox/commons-dev/201505.mbox/%3CCAB917RKab4oO_OMa_FcMuEegZwKA%2BNQNNOjjMnYE9QDGtNHXdg%40mail.gmail.com%3E
>>>>
>>>> :)
>
Re: Thread safety and OptionBuilder
Posted by Pascal Schumacher <pa...@gmx.net>.
Hi Jacopo,
I pushed the change to my clone of the groovy repository on github. This
is what I did:
https://github.com/PascalSchumacher/incubator-groovy/commit/205a0b7188612b386038bcd01b4a1184e51d9be7
- Pascal
Am 06.05.2015 um 14:44 schrieb Jacopo Cappellato:
> Hi Pascal,
>
> what is the easiest way to replace it in my dev box? I would like to test the same and try to migrate some of the code to the new API.
>
> Thanks,
>
> Jacopo
>
> On May 5, 2015, at 7:22 PM, Pascal Schumacher <pa...@gmx.net> wrote:
>
>> I replaced the commons-cli 1.2 dependency of Groovy with the 1.3 rc and ran some checks:
>>
>> - Build passes
>> - Tests pass
>> - Quick manual check using groovysh successful
>>
>> Am 03.05.2015 um 20:43 schrieb Pascal Schumacher:
>>> Am 03.05.2015 um 13:03 schrieb Benedikt Ritter:
>>>> Good point. I could try to do the pre release clean up later today and get an RC out this week.
>>> Thanks for creating the RC:
>>>
>>> https://mail-archives.apache.org/mod_mbox/commons-dev/201505.mbox/%3CCAB917RKab4oO_OMa_FcMuEegZwKA%2BNQNNOjjMnYE9QDGtNHXdg%40mail.gmail.com%3E
>>>
>>> :)
Re: Thread safety and OptionBuilder
Posted by Jacopo Cappellato <ja...@gmail.com>.
Hi Pascal,
what is the easiest way to replace it in my dev box? I would like to test the same and try to migrate some of the code to the new API.
Thanks,
Jacopo
On May 5, 2015, at 7:22 PM, Pascal Schumacher <pa...@gmx.net> wrote:
> I replaced the commons-cli 1.2 dependency of Groovy with the 1.3 rc and ran some checks:
>
> - Build passes
> - Tests pass
> - Quick manual check using groovysh successful
>
> Am 03.05.2015 um 20:43 schrieb Pascal Schumacher:
>> Am 03.05.2015 um 13:03 schrieb Benedikt Ritter:
>>> Good point. I could try to do the pre release clean up later today and get an RC out this week.
>>
>> Thanks for creating the RC:
>>
>> https://mail-archives.apache.org/mod_mbox/commons-dev/201505.mbox/%3CCAB917RKab4oO_OMa_FcMuEegZwKA%2BNQNNOjjMnYE9QDGtNHXdg%40mail.gmail.com%3E
>>
>> :)
>
Re: Thread safety and OptionBuilder
Posted by Pascal Schumacher <pa...@gmx.net>.
I replaced the commons-cli 1.2 dependency of Groovy with the 1.3 rc and
ran some checks:
- Build passes
- Tests pass
- Quick manual check using groovysh successful
Am 03.05.2015 um 20:43 schrieb Pascal Schumacher:
> Am 03.05.2015 um 13:03 schrieb Benedikt Ritter:
>> Good point. I could try to do the pre release clean up later today
>> and get an RC out this week.
>
> Thanks for creating the RC:
>
> https://mail-archives.apache.org/mod_mbox/commons-dev/201505.mbox/%3CCAB917RKab4oO_OMa_FcMuEegZwKA%2BNQNNOjjMnYE9QDGtNHXdg%40mail.gmail.com%3E
>
>
> :)
Re: Thread safety and OptionBuilder
Posted by Pascal Schumacher <pa...@gmx.net>.
Am 03.05.2015 um 13:03 schrieb Benedikt Ritter:
> Good point. I could try to do the pre release clean up later today and
> get an RC out this week.
Thanks for creating the RC:
https://mail-archives.apache.org/mod_mbox/commons-dev/201505.mbox/%3CCAB917RKab4oO_OMa_FcMuEegZwKA%2BNQNNOjjMnYE9QDGtNHXdg%40mail.gmail.com%3E
:)
Re: Thread safety and OptionBuilder
Posted by Benedikt Ritter <br...@apache.org>.
2015-05-03 12:02 GMT+02:00 Pascal Schumacher <pa...@gmx.net>:
> Am 03.05.2015 um 10:42 schrieb Benedikt Ritter:
>
>>
>> Nope, that's not how Apache works (at least in my opinion :-) ). I've
>> planned to push a release but then good caught up with other stuff. Here
>> is the list of things that should be resolved for 1.3 [1]. If people jump
>> in and help by providing patches (or PR via github [2]). This will speed up
>> the process of pushing out a new release. Now that groovy is part of the
>> Apache family, I would love to see this kind of inter-project collaboration
>> happen.
>>
>> br,
>> Benedikt
>>
>> [1]
>> https://issues.apache.org/jira/browse/CLI-179?jql=project%20%3D%20CLI%20AND%20fixVersion%20%3D%201.3%20AND%20resolution%20%3D%20Unresolved%20ORDER%20BY%20due%20ASC%2C%20priority%20DESC%2C%20created%20ASC
>> [2] https://github.com/apache/commons-cli
>>
>
> Hi Benedikt,
>
> thanks for the information. :)
>
> For course I hope that somebody steps up and helps closing these issues.
>
> From a short glance at the issues I'm not convinced that they are really
> blocking a release.
>
> The last release of commons cli was on 2009-03-19 and there are already a
> lot of bugs fixed and features added:
>
>
> https://issues.apache.org/jira/browse/CLI-193?jql=project%20%3D%20CLI%20AND%20resolution%20%3D%20Fixed%20AND%20fixVersion%20%3D%201.3%20ORDER%20BY%20due%20ASC%2C%20priority%20DESC%2C%20created%20ASC
>
> People have been waiting for a release for a very long time now. So I
> would suggest postponing the issues to 1.4 and going ahead with the release
> of 1.3. :) (Of course that's not for me to decide.)
Good point. I could try to do the pre release clean up later today and get
an RC out this week.
>
>
> - Pascal
>
--
http://people.apache.org/~britter/
http://www.systemoutprintln.de/
http://twitter.com/BenediktRitter
http://github.com/britter
Re: Thread safety and OptionBuilder
Posted by Pascal Schumacher <pa...@gmx.net>.
Am 03.05.2015 um 10:42 schrieb Benedikt Ritter:
>
> Nope, that's not how Apache works (at least in my opinion :-) ). I've
> planned to push a release but then good caught up with other stuff.
> Here is the list of things that should be resolved for 1.3 [1]. If
> people jump in and help by providing patches (or PR via github [2]).
> This will speed up the process of pushing out a new release. Now that
> groovy is part of the Apache family, I would love to see this kind of
> inter-project collaboration happen.
>
> br,
> Benedikt
>
> [1]
> https://issues.apache.org/jira/browse/CLI-179?jql=project%20%3D%20CLI%20AND%20fixVersion%20%3D%201.3%20AND%20resolution%20%3D%20Unresolved%20ORDER%20BY%20due%20ASC%2C%20priority%20DESC%2C%20created%20ASC
>
> [2] https://github.com/apache/commons-cli
Hi Benedikt,
thanks for the information. :)
For course I hope that somebody steps up and helps closing these issues.
From a short glance at the issues I'm not convinced that they are
really blocking a release.
The last release of commons cli was on 2009-03-19 and there are already
a lot of bugs fixed and features added:
https://issues.apache.org/jira/browse/CLI-193?jql=project%20%3D%20CLI%20AND%20resolution%20%3D%20Fixed%20AND%20fixVersion%20%3D%201.3%20ORDER%20BY%20due%20ASC%2C%20priority%20DESC%2C%20created%20ASC
People have been waiting for a release for a very long time now. So I
would suggest postponing the issues to 1.4 and going ahead with the
release of 1.3. :) (Of course that's not for me to decide.)
- Pascal
Re: Thread safety and OptionBuilder
Posted by Benedikt Ritter <br...@apache.org>.
Hi,
2015-05-03 8:59 GMT+02:00 Pascal Schumacher <pa...@gmx.net>:
> Am 03.05.2015 um 08:26 schrieb Jochen Theodorou:
>
>> Am 02.05.2015 12:51, schrieb Jacopo Cappellato:
>> [...]
>>
>>> My question is: is this a concern/known issue for Groovy?
>>>
>>
>> I am not aware of an issue with that problem
>>
>> what is the best way to address the issue?
>>>
>>
>> the best way would be to have a 1.3 release of commons-cli. I don't how
>> any synchronization higher up the chain could gurantee thread safety.
>>
>> Should we get in touch with the Commons CLI community to know more about
>>> their plans or should me look for a different approach?
>>>
>>
>> Well, I did hear they have become more active again and there might be
>> even a 1.3 release soon. The release 2 years ago happened mostly because of
>> people of the Groovy community giving some pressure. The project is usually
>> very very very quite/inactive. But things may have changed. Feel free to
>> approach people there.
>>
>>
> They planned to do a release of 1.3 soon:
> https://mail-archives.apache.org/mod_mbox/commons-dev/201504.mbox/%3CCAB917RK8f1ARmSu-1GAXVe79EdgSahoFLMS40nAmzKsxgKTXKg%40mail.gmail.com%3E
>
> but that was nearly a month ago and it seem like nothing happend. :( I
> hope they will release soon. Maybe some poking would help. ;(
>
Nope, that's not how Apache works (at least in my opinion :-) ). I've
planned to push a release but then good caught up with other stuff. Here is
the list of things that should be resolved for 1.3 [1]. If people jump in
and help by providing patches (or PR via github [2]). This will speed up
the process of pushing out a new release. Now that groovy is part of the
Apache family, I would love to see this kind of inter-project collaboration
happen.
br,
Benedikt
[1]
https://issues.apache.org/jira/browse/CLI-179?jql=project%20%3D%20CLI%20AND%20fixVersion%20%3D%201.3%20AND%20resolution%20%3D%20Unresolved%20ORDER%20BY%20due%20ASC%2C%20priority%20DESC%2C%20created%20ASC
[2] https://github.com/apache/commons-cli
--
http://people.apache.org/~britter/
http://www.systemoutprintln.de/
http://twitter.com/BenediktRitter
http://github.com/britter
Re: Thread safety and OptionBuilder
Posted by Pascal Schumacher <pa...@gmx.net>.
Am 03.05.2015 um 08:26 schrieb Jochen Theodorou:
> Am 02.05.2015 12:51, schrieb Jacopo Cappellato:
> [...]
>> My question is: is this a concern/known issue for Groovy?
>
> I am not aware of an issue with that problem
>
>> what is the best way to address the issue?
>
> the best way would be to have a 1.3 release of commons-cli. I don't
> how any synchronization higher up the chain could gurantee thread safety.
>
>> Should we get in touch with the Commons CLI community to know more
>> about their plans or should me look for a different approach?
>
> Well, I did hear they have become more active again and there might be
> even a 1.3 release soon. The release 2 years ago happened mostly
> because of people of the Groovy community giving some pressure. The
> project is usually very very very quite/inactive. But things may have
> changed. Feel free to approach people there.
>
They planned to do a release of 1.3 soon:
https://mail-archives.apache.org/mod_mbox/commons-dev/201504.mbox/%3CCAB917RK8f1ARmSu-1GAXVe79EdgSahoFLMS40nAmzKsxgKTXKg%40mail.gmail.com%3E
but that was nearly a month ago and it seem like nothing happend. :( I
hope they will release soon. Maybe some poking would help. ;(
Re: Thread safety and OptionBuilder
Posted by Jochen Theodorou <bl...@gmx.org>.
Am 02.05.2015 12:51, schrieb Jacopo Cappellato:
[...]
> My question is: is this a concern/known issue for Groovy?
I am not aware of an issue with that problem
> what is the best way to address the issue?
the best way would be to have a 1.3 release of commons-cli. I don't how
any synchronization higher up the chain could gurantee thread safety.
> Should we get in touch with the Commons CLI community to know more about their plans or should me look for a different approach?
Well, I did hear they have become more active again and there might be
even a 1.3 release soon. The release 2 years ago happened mostly because
of people of the Groovy community giving some pressure. The project is
usually very very very quite/inactive. But things may have changed. Feel
free to approach people there.
bye blackdrag
--
Jochen "blackdrag" Theodorou
blog: http://blackdragsview.blogspot.com/
Re: Thread safety and OptionBuilder
Posted by Jacopo Cappellato <ja...@gmail.com>.
On May 3, 2015, at 3:34 AM, KARR, DAVID <dk...@att.com> wrote:
>
> Why exactly do you believe that synchronizing the static method is not enough to "make the Groovy code thread safe" (ignoring the lack of specificity in that statement).
>
> If OptionsBuilder is truly not thread-safe, if you can guarantee that this class is the only one making use of OptionsBuilder in the application, and this static synchronized method is the only method in the class using OptionsBuilder, then that seems like reasonable protection.
Hi David,
I am saying that synchronizing the static method is not enough because I don't see how we can guarantee that this class is the only one making use of OptionBuilder for these reasons:
* the static method in the class is called by a public method in the same class
* other classes within the same JVM could use OptionsBuilder: OptionsBuilder is used by other code in Groovy but it could also be used by Groovy scripts written by users
Regards,
Jacopo
RE: Thread safety and OptionBuilder
Posted by "KARR, DAVID" <dk...@att.com>.
> -----Original Message-----
> From: Jacopo Cappellato [mailto:jacopo.cappellato@gmail.com]
> Sent: Saturday, May 02, 2015 3:52 AM
> To: dev@groovy.incubator.apache.org
> Subject: Thread safety and OptionBuilder
>
> Hi all,
>
> in the GroovyMain class there is the following code:
>
> /**
> * Build the options parser. Has to be synchronized because of the
> way Options are constructed.
> *
> * @return an options parser.
> */
> private static synchronized Options buildOptions() {
> Options options = new Options();
> options.addOption(OptionBuilder.hasArg()....
> ...
>
> The JavaDoc comment says:
>
> "Has to be synchronized because of the way Options are
> constructed."
>
> but unfortunately this is not enough to make the Groovy code thread
> safe.
> In fact the current version of Commons CLI used by Groovy is 1.2,
> which is the latest stable release, and in this version the
> OptionBuilder class is clearly not meant to be used by more than
> one thread (there are several static fields modified by the builder
> methods); the issue has been reported a few times (e.g.
> https://issues.apache.org/jira/browse/CLI-209) and has been fixed
> by deprecating the class in favor of a new Option.builder() method:
>
> https://commons.apache.org/proper/commons-
> cli/apidocs/org/apache/commons/cli/OptionBuilder.html
>
> Unfortunately I don't know if Commons-CLI 1.3 will be released soon
> (1.2. was released 2 years ago). And the OptionBuilder class is
> used in a few classes in Groovy.
>
> My question is: is this a concern/known issue for Groovy? what is
> the best way to address the issue? Should we get in touch with the
> Commons CLI community to know more about their plans or should me
> look for a different approach? With this information I will try to
> help to move in the direction you feel is the best.
I know nothing about this issue, I'm just reading what you've written here and interpreting it.
Why exactly do you believe that synchronizing the static method is not enough to "make the Groovy code thread safe" (ignoring the lack of specificity in that statement).
If OptionsBuilder is truly not thread-safe, if you can guarantee that this class is the only one making use of OptionsBuilder in the application, and this static synchronized method is the only method in the class using OptionsBuilder, then that seems like reasonable protection.