You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Thomas Herre (JIRA)" <ji...@apache.org> on 2010/09/27 16:27:33 UTC

[jira] Created: (CLI-209) OptionBuilder is not thread-safe

OptionBuilder is not thread-safe
--------------------------------

                 Key: CLI-209
                 URL: https://issues.apache.org/jira/browse/CLI-209
             Project: Commons CLI
          Issue Type: Bug
          Components: CLI-1.x
    Affects Versions: 1.2, 1.1, 1.0
         Environment: Windows XP, Java 6
            Reporter: Thomas Herre


Class OptionBuilder is not thread-safe.
It uses one OptionBuilder instance without synchronizing access to it.
If more than one thread uses OptionBuilder, that can result in invalid Options (suppose a scheduler component).

Don't know if this should be fixed. At least the Javadoc should mention the fact that the class is not thread-safe.
As a workaround, use constructor of class Option.


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CLI-209) OptionBuilder is not thread-safe

Posted by "Sebb (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CLI-209?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12915694#action_12915694 ] 

Sebb commented on CLI-209:
--------------------------

Another way to fix the class would be to turn all the static variables into instance variables, and add parallel instance methods. The static methods would then use a static singleton instance. That would allow use from multiple threads without the overhead of ThreadLocal.

The library would still not be thread-safe, but at least it could be used by multiple threads in parallel so long as each thread had its own instances.

> OptionBuilder is not thread-safe
> --------------------------------
>
>                 Key: CLI-209
>                 URL: https://issues.apache.org/jira/browse/CLI-209
>             Project: Commons CLI
>          Issue Type: Bug
>          Components: CLI-1.x
>    Affects Versions: 1.0, 1.1, 1.2
>         Environment: Windows XP, Java 6
>            Reporter: Thomas Herre
>
> Class OptionBuilder is not thread-safe.
> It uses one OptionBuilder instance without synchronizing access to it.
> If more than one thread uses OptionBuilder, that can result in invalid Options (suppose a scheduler component).
> Don't know if this should be fixed. At least the Javadoc should mention the fact that the class is not thread-safe.
> As a workaround, use constructor of class Option.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (CLI-209) OptionBuilder is not thread-safe

Posted by "Emmanuel Bourg (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CLI-209?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Emmanuel Bourg updated CLI-209:
-------------------------------

    Environment:     (was: Windows XP, Java 6)
       Priority: Minor  (was: Major)

> OptionBuilder is not thread-safe
> --------------------------------
>
>                 Key: CLI-209
>                 URL: https://issues.apache.org/jira/browse/CLI-209
>             Project: Commons CLI
>          Issue Type: Bug
>          Components: CLI-1.x
>    Affects Versions: 1.0, 1.1, 1.2
>            Reporter: Thomas Herre
>            Priority: Minor
>
> Class OptionBuilder is not thread-safe.
> It uses one OptionBuilder instance without synchronizing access to it.
> If more than one thread uses OptionBuilder, that can result in invalid Options (suppose a scheduler component).
> Don't know if this should be fixed. At least the Javadoc should mention the fact that the class is not thread-safe.
> As a workaround, use constructor of class Option.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Issue Comment Edited: (CLI-209) OptionBuilder is not thread-safe

Posted by "Sebb (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CLI-209?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12915331#action_12915331 ] 

Sebb edited comment on CLI-209 at 9/27/10 11:46 AM:
----------------------------------------------------

In fact the OptionBuilder code is at best serially re-usable. 

Even then, the second thread would need to ensure that reset() was called before starting to use the class, because the lack of synch. means that the static variables updated by one thread may not be published to the second thread - i.e. the second thread might see stale values even though the first thread called reset() as part of create(). And of course the threads would have to ensure that they took it in turns to use the class.

Note that Option is not thread-safe either (settters/getters not synch.), however if the Option instance is confined to a single thread it would be OK.

Many of the other classes are not thread-safe either, as they have mutable variables that aren't volatile or synch.
But if confined to a thread, they would be OK.

Unless the documentation says otherwise, it's not safe to assume that a lbrary is thread-safe.

But I agree that the OptionBuilder is a particular problem.

It could be fixed by using ThreadLocal for all the static variables. But is it worth the effort?

      was (Author: sebb@apache.org):
    In fact the OptionBuilder class would be very tricky (perhaps impossible) to make thread-safe. 
The code is at best serially re-usable. 

Even then, the second thread would need to ensure that reset() was called before starting to use the class, because the lack of synch. means that the static variables updated by one thread may not be published to the second thread - i.e. the second thread might see stale values even though the first thread called reset() as part of create(). And of course the threads would have to ensure that they took it in turns to use the class.

Note that Option is not thread-safe either (settters/getters not synch.), however if the Option instance is confined to a single thread it would be OK.

Many of the other classes are not thread-safe either, as they have mutable variables that aren't volatile or synch.
But if confined to a thread, they would be OK.

Unless the documentation says otherwise, it's not safe to assume that a lbrary is thread-safe.

But I agree that the OptionBuilder is a particular problem.

It could be fixed by using ThreadLocal for all the static variables. But is it worth the effort?
  
> OptionBuilder is not thread-safe
> --------------------------------
>
>                 Key: CLI-209
>                 URL: https://issues.apache.org/jira/browse/CLI-209
>             Project: Commons CLI
>          Issue Type: Bug
>          Components: CLI-1.x
>    Affects Versions: 1.0, 1.1, 1.2
>         Environment: Windows XP, Java 6
>            Reporter: Thomas Herre
>
> Class OptionBuilder is not thread-safe.
> It uses one OptionBuilder instance without synchronizing access to it.
> If more than one thread uses OptionBuilder, that can result in invalid Options (suppose a scheduler component).
> Don't know if this should be fixed. At least the Javadoc should mention the fact that the class is not thread-safe.
> As a workaround, use constructor of class Option.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CLI-209) OptionBuilder is not thread-safe

Posted by "Emmanuel Bourg (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CLI-209?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12915662#action_12915662 ] 

Emmanuel Bourg commented on CLI-209:
------------------------------------

Just curious, what is the use case that would require making CLI thread safe? A typical application uses CLI on its main thread to parse the command line parameters, that's far from a multi thread scenario.

> OptionBuilder is not thread-safe
> --------------------------------
>
>                 Key: CLI-209
>                 URL: https://issues.apache.org/jira/browse/CLI-209
>             Project: Commons CLI
>          Issue Type: Bug
>          Components: CLI-1.x
>    Affects Versions: 1.0, 1.1, 1.2
>         Environment: Windows XP, Java 6
>            Reporter: Thomas Herre
>
> Class OptionBuilder is not thread-safe.
> It uses one OptionBuilder instance without synchronizing access to it.
> If more than one thread uses OptionBuilder, that can result in invalid Options (suppose a scheduler component).
> Don't know if this should be fixed. At least the Javadoc should mention the fact that the class is not thread-safe.
> As a workaround, use constructor of class Option.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (CLI-209) OptionBuilder is not thread-safe

Posted by "Emmanuel Bourg (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CLI-209?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Emmanuel Bourg resolved CLI-209.
--------------------------------

       Resolution: Fixed
    Fix Version/s: 1.3

I updated the Javadoc as suggested by Thomas.

Ideally I'd like to rewrite OptionBuilder with non static methods, beside the thread safety issue this cause my IDE to show a lot of ugly warnings, that's not a pleasant experience. Due to the incompatible nature of this modification it will have to wait for a major release. We might also deprecate the class and rewrite it under a new name, for example OptionsBuilder.

> OptionBuilder is not thread-safe
> --------------------------------
>
>                 Key: CLI-209
>                 URL: https://issues.apache.org/jira/browse/CLI-209
>             Project: Commons CLI
>          Issue Type: Bug
>          Components: CLI-1.x
>    Affects Versions: 1.0, 1.1, 1.2
>            Reporter: Thomas Herre
>            Priority: Minor
>             Fix For: 1.3
>
>
> Class OptionBuilder is not thread-safe.
> It uses one OptionBuilder instance without synchronizing access to it.
> If more than one thread uses OptionBuilder, that can result in invalid Options (suppose a scheduler component).
> Don't know if this should be fixed. At least the Javadoc should mention the fact that the class is not thread-safe.
> As a workaround, use constructor of class Option.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CLI-209) OptionBuilder is not thread-safe

Posted by "Sebb (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CLI-209?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12915331#action_12915331 ] 

Sebb commented on CLI-209:
--------------------------

In fact the OptionBuilder class would be very tricky (perhaps impossible) to make thread-safe. 
The code is at best serially re-usable. 

Even then, the second thread would need to ensure that reset() was called before starting to use the class, because the lack of synch. means that the static variables updated by one thread may not be published to the second thread - i.e. the second thread might see stale values even though the first thread called reset() as part of create(). And of course the threads would have to ensure that they took it in turns to use the class.

Note that Option is not thread-safe either (settters/getters not synch.), however if the Option instance is confined to a single thread it would be OK.

Many of the other classes are not thread-safe either, as they have mutable variables that aren't volatile or synch.
But if confined to a thread, they would be OK.

Unless the documentation says otherwise, it's not safe to assume that a lbrary is thread-safe.

But I agree that the OptionBuilder is a particular problem.

It could be fixed by using ThreadLocal for all the static variables. But is it worth the effort?

> OptionBuilder is not thread-safe
> --------------------------------
>
>                 Key: CLI-209
>                 URL: https://issues.apache.org/jira/browse/CLI-209
>             Project: Commons CLI
>          Issue Type: Bug
>          Components: CLI-1.x
>    Affects Versions: 1.0, 1.1, 1.2
>         Environment: Windows XP, Java 6
>            Reporter: Thomas Herre
>
> Class OptionBuilder is not thread-safe.
> It uses one OptionBuilder instance without synchronizing access to it.
> If more than one thread uses OptionBuilder, that can result in invalid Options (suppose a scheduler component).
> Don't know if this should be fixed. At least the Javadoc should mention the fact that the class is not thread-safe.
> As a workaround, use constructor of class Option.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.