Skip to content
This repository has been archived by the owner on Jun 29, 2020. It is now read-only.

docker_entrypoint script incorrectly inserts the tmpdir property #38

Closed
gregw opened this issue Jun 28, 2016 · 5 comments
Closed

docker_entrypoint script incorrectly inserts the tmpdir property #38

gregw opened this issue Jun 28, 2016 · 5 comments

Comments

@gregw
Copy link
Contributor

gregw commented Jun 28, 2016

The current CMD has -jar start.jar before -Djava.io.tmpdir=.... This forces start.jar to always fork a new JVM.

We should consider putting the -D first, so a new JVM is only required if a module is enabled that needs additional JVM args (eg ALPN).

@gregw
Copy link
Contributor Author

gregw commented Jun 28, 2016

This issue is a misunderstanding! The issue pointed at by @md5 was not that the -D was after the -jar start.jar, but that in the entry point it is between the -jar and the start.jar. That is just broken!

@gregw gregw changed the title Jetty tmpdir forces start.jar to fork new JVM docker_entrypoint script incorrectly inserts the tmpdir property Jun 28, 2016
@md5
Copy link
Member

md5 commented Jun 29, 2016

It looks like it's actually not broken...

If you look at the source for the java command, it doesn't strictly check that the JAR file name comes after -jar. All it does is check that there is at least one argument after -jar, but it doesn't immediately capture that argument as the JAR file name. When it finds -jar and sees that there are still arguments after it, it continues in a loop that checks for arguments starting with "-". If it sees them, it processes them as normal. Then, when it finds the first argument that doesn't start with "-", the first of those arguments is treated as the JAR file name.

See here: https://github.com/openjdk-mirror/jdk/blob/d564b7c51ace5e0e1dbf3bb2f50483f0648529d2/src/share/bin/java.c#L1072

@gregw
Copy link
Contributor Author

gregw commented Jun 29, 2016

whew! I knew I had tested that functionality! So not broken, but not desirable either.

@md5
Copy link
Member

md5 commented Jun 29, 2016

I'm working on a PR

@md5
Copy link
Member

md5 commented Jun 29, 2016

Fixed in #39

@md5 md5 closed this as completed Jun 29, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants