On Wed, 12 Nov 2025 23:11:18 GMT, Alexey Semenyuk <[email protected]> wrote:
>> Use "JOpt Simple" from >> [jdk.internal.joptsimple](https://github.com/openjdk/jdk/tree/master/src/jdk.internal.opt/share/classes/jdk/internal/joptsimple) >> package to parse jpackage command line. >> >> All command-line parsing code is placed in a new "jdk.jpackage.internal.cli" >> package with 92% unit test coverage. >> >> ### Error reporting improved >> >> 1. In case of multiple command-line errors, all are reported, unlike >> previously, only the first one was reported. >> >> Command line (Windows): >> >> jpackage --linux-shortcut --mac-package-name foo -p m1 --linux-menu-group >> grp -p m2 --app-image dir >> >> Old error output: >> >> Error: Option [--linux-shortcut] is not valid on this platform >> >> >> New error output: >> >> Error: Option [--linux-shortcut] is not valid on this platform >> Error: Option [--mac-package-name] is not valid on this platform >> Error: Option [-p] is not valid with type [exe] >> Error: Option [--linux-menu-group] is not valid on this platform >> >> >> 2. Fix misleading error messages. >> >> Command line (Windows): >> >> jpackage --input no --main-jar no.jar >> >> Old error output: >> >> jdk.jpackage.internal.model.ConfigException: The configured main jar does >> not exist no.jar in the input directory >> >> >> New error output: >> >> The value "no" provided for parameter --input is not a directory >> >> >> >> >> ### Help output fixed >> >> Options in the original help output were out of order. On macOS, options >> were placed in wrong sections. There were trailing whitespaces. >> >> The old help output is captured in the >> cd7bca2bb665556f314170c81129ef53de91f135 commit. >> >> The reordered and filtered old help output is captured in the >> 10dc3792e6896cfa4bbe8693ee33e4c5df45d952 commit. >> >> Help output in this PR is captured in the >> 58c2d944e2e14b1cf35786162ad2a5f9a8ccfee6 commit. Use it to see the diff >> between the new and old filtered and reordered help output. >> >> ### Functional changes >> >> Old tool provider implementation >> [jdk.jpackage.internal.JPackageToolProvider](https://github.com/openjdk/jdk/blob/5fccabff15ae8bcc3d03156fa331bbc0fefb0cbe/src/jdk.jpackage/share/classes/jdk/jpackage/internal/JPackageToolProvider.java#L48) >> had lousy thread-safety protection that didn't work when multiple instances >> of jpackage tool provider are created and invoked asynchronously. This patch >> fixes this issue. It is safe to invoke the same jpackage tool provider >> instance asynchronously, and also safe to invoke multiple instances of >> jpackage tool provider. >> >> Like other JD... > > Alexey Semenyuk has updated the pull request incrementally with one > additional commit since the last revision: > > MainResources.properties: remove unreferenced L10N key There were a few synchronization issues with jpackage. The major one was the use of static member fields throughout the codebase. Andy attempted to fix it with [JDK-8223322](https://bugs.openjdk.org/browse/JDK-8223322), but due to other synchronization issues, the fix for [JDK-8223322](https://bugs.openjdk.org/browse/JDK-8223322) https://github.com/openjdk/jdk/pull/1829 was never integrated. Instead, he applied a smaller-scope fix [JDK-8259238](https://bugs.openjdk.org/browse/JDK-8259238) that fixed the use of static member fields. However, the "synchronized" keyword in [JPackageToolProvider.java](https://github.com/openjdk/jdk/blob/5fccabff15ae8bcc3d03156fa331bbc0fefb0cbe/src/jdk.jpackage/share/classes/jdk/jpackage/internal/JPackageToolProvider.java#L48) was still in place. I don't know what the intent of this "synchronized" is. It doesn't apply to a global object but to the instance of the "jdk.jpackage.internal.JPackageToolProvider" class. Looks like the assumption was that the sy stem would create only a single instance of a specific tool provider. There is no such guarantee, though. If multiple instances of the "jdk.jpackage.internal.JPackageToolProvider" class are created and used asynchronously, the "synchronized" keyword doesn't do what it is supposed to. The new tool provider implementation in the "jdk.jpackage.internal.cli.Main" class doesn't use "synchronized" keyword. Command-line parsing code doesn't use global variables; it is thread-safe by design, rather than having thread safety as a patch. ------------- PR Comment: https://git.openjdk.org/jdk/pull/28163#issuecomment-3534396848
