Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

picocli throws a big StackOverflowError #1680

Closed
zappee opened this issue May 15, 2022 · 5 comments
Closed

picocli throws a big StackOverflowError #1680

zappee opened this issue May 15, 2022 · 5 comments
Labels
Milestone

Comments

@zappee
Copy link

zappee commented May 15, 2022

I am working on a Java CLI application and I get a picocli StackOverflowError.
This is a strange error and really hard to reproduce. I have tried to create a simple class that demonstrates the issue but that is not easy so the best is if you use the original project.

Steps to reproduce the StackOverflowError:

git clone https://github.com/zappee/portfolio-analyzer.git
cd portfolio-analyzer/
git checkout remotes/origin/feature/portfolio-summary
JAVA_HOME=/usr/lib/jvm/java-18-openjdk-18.0.1.0.10-1.rolling.el7.x86_64
mvn clean package
java -jar target/portfolio-analyzer-0.1.13.jar price -i bb -d COINBASE_PRO -k aa -p aa -e aa

result:

$ java -jar target/portfolio-analyzer-0.1.13.jar price -i bb -d COINBASE_PRO -k aa -p aa -e aa
Exception in thread "main" java.lang.StackOverflowError
	at picocli.CommandLine$ParseResult$GroupMatchContainer.addMatch(CommandLine.java:12517)
	at picocli.CommandLine$ParseResult$GroupMatchContainer.complete(CommandLine.java:12532)
	at picocli.CommandLine$ParseResult$GroupMatchContainer.addMatch(CommandLine.java:12520)
	at picocli.CommandLine$ParseResult$GroupMatchContainer.complete(CommandLine.java:12532)
	at picocli.CommandLine$ParseResult$GroupMatchContainer.addMatch(CommandLine.java:12520)
	at picocli.CommandLine$ParseResult$GroupMatchContainer.complete(CommandLine.java:12532)
	at picocli.CommandLine$ParseResult$GroupMatchContainer.addMatch(CommandLine.java:12520)
	at picocli.CommandLine$ParseResult$GroupMatchContainer.complete(CommandLine.java:12532)
	at picocli.CommandLine$ParseResult$GroupMatchContainer.addMatch(CommandLine.java:12520)
	at picocli.CommandLine$ParseResult$GroupMatchContainer.complete(CommandLine.java:12532)
	at picocli.CommandLine$ParseResult$GroupMatchContainer.addMatch(CommandLine.java:12520)
	at picocli.CommandLine$ParseResult$GroupMatchContainer.complete(CommandLine.java:12532)
	at picocli.CommandLine$ParseResult$GroupMatchContainer.addMatch(CommandLine.java:12520)
	at picocli.CommandLine$ParseResult$GroupMatchContainer.complete(CommandLine.java:12532)
	at picocli.CommandLine$ParseResult$GroupMatchContainer.addMatch(CommandLine.java:12520)
...

The picocli works great, the error only appears in this specific use case: price -i bb -d COINBASE_PRO -k aa -p aa -e aa
I hope that you can reproduce this error.

@remkop
Copy link
Owner

remkop commented May 15, 2022

Thanks for raising this, that doesn't look good!
I'll take a look when I get to my PC.

@remkop remkop added this to the 4.7 milestone May 16, 2022
@remkop remkop added type: bug 🐛 theme: arg-group An issue or change related to argument groups labels May 16, 2022
@remkop
Copy link
Owner

remkop commented May 17, 2022

Quick update:
I have been able to reproduce the issue with your project (thank you).
Have not had enough time to look in detail at the cause.

@zappee
Copy link
Author

zappee commented May 17, 2022

Thanks for the update. I re-arranged a little bit the command line interface and that way StackOverflowError dissapeared.

@remkop
Copy link
Owner

remkop commented May 27, 2022

Hi @zappee, I finally had a chance to spend some time stepping through this in a debugger.
I think I found the cause:

The PriceCommand class has this ArgGroup:

    /**
     * Coinbase PRO API configuration.
     */
    @CommandLine.ArgGroup(
            exclusive = false,
            multiplicity = "0", // <---------------- this is not good
            heading = "%nCoinbase PRO API:%n")
    private final CoinbaseDataSourceArgGroup coinbaseArgGroup = new CoinbaseDataSourceArgGroup();

Note the multiplicity = "0" value: this means that the group can be specified zero times!
This is not a valid value for the multiplicity range, but picocli unfortunately does not validate this, and gets stuck trying to enforce that constraint...

From the beginning, there are zero matches, so the max multiplicity (zero) is reached, and picocli then gets in an infinite loop...

Exception in thread "main" java.lang.StackOverflowError
        at picocli.CommandLine$ParseResult$GroupMatchContainer.isMaxMultiplicityReached(CommandLine.java:12540)
        at picocli.CommandLine$ParseResult$GroupMatchContainer.addMatch(CommandLine.java:12518)
        at picocli.CommandLine$ParseResult$GroupMatchContainer.complete(CommandLine.java:12532)
        at picocli.CommandLine$ParseResult$GroupMatchContainer.addMatch(CommandLine.java:12520)
        at picocli.CommandLine$ParseResult$GroupMatchContainer.complete(CommandLine.java:12532)

Great catch, thank you for raising this!

On the application side, this can be avoided by specifying multiplicity = "0..1", or multiplicity = "0..*", or multiplicity = "1" or whatever other non-zero value makes sense for the application.

On the library side, I will look at adding validation for the multiplicity range: it does not make sense to define ArgGroups that can be specified zero times (never) by the user.

Also, there may still be a potential for an infinite loop in that code, even when max multiplicity is larger than zero, I need to investigate that further.

@zappee
Copy link
Author

zappee commented May 30, 2022

Thx for the response. As I remember during the refactor that I did I modified the multiplicity value as well. So maybe this is why the issue has dissapeared. Thx.

@remkop remkop closed this as completed in c1fd7fd Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants