On Tue, 24 Mar 2026 12:04:18 GMT, David Beaumont <[email protected]> wrote:

>> Implementation of preview-mode support for jimage modules file, migrated 
>> from Valhalla related work (see JDK-8352750).
>> 
>> This PR (the first of several) migrates work from Valhalla (lworld) to the 
>> JDK mainline repository in relation to "preview mode" support. It affects 
>> the creation and reading of the jimage file, both in Java 
>> (BasicImageReader/ImageReader) and C++ (imageFile.xpp/jimage.xpp).
>> 
>> Preview mode is a mechanism by which alternate version of JDK class files 
>> and resources can be made available for class loading and reflection when 
>> the '--enable-preview' flag is passed to the runtime.
>> 
>> Alternate classes/resource appear in each module under the:
>> 
>> /<module>/META-INF/preview/<path-to>/<resource-or-class>
>> 
>> and replace the original:
>> 
>> /<module>/<path-to>/<resource-or-class>
>> 
>> files when preview mode is enabled.
>> 
>> While initially useful for Valhalla work, this mechanism will be used for 
>> other cases where preview features (in the JEP 12 sense) require alternate 
>> classes/resources to be provided. None of the changes in this (or the 
>> follow-up PRs) are Valhalla specific.
>> 
>> In this PR:
>> * the writing of jimage files is modified to recognize and handle preview 
>> mode paths
>> * flags in the jimage file are added or modified to support preview mode 
>> efficiently
>> * (C++) the class loader is modified to permit reading preview versions of 
>> classes
>> * (Java) the image reader and associated JRT file-system classes are 
>> modified to permit reading preview files
>> * unit tests are added to ensure preview mode works as expected when enabled
>> * (temporary) any code calling into the affected API (other than tests) 
>> specifies that preview mode is disabled
>> 
>> Future PRs will add the plumbing to enable preview mode correctly, but with 
>> the PR there should be no observable change in behaviour (especially since 
>> no preview classes or resources are being supplied at this point).
>
> David Beaumont has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 12 commits:
> 
>  - Merge branch 'master' into jimage_preview_mode
>  - rename jimage_exists to jimage_is_open
>  - Feedback tweaks
>  - Feedback tweaks
>  - More feedback tweaks
>  - Updated copyright
>  - Feedback changes
>  - Merge branch 'master' into jimage_preview_mode
>  - Merge branch 'master' into jimage_preview_mode
>  - undo exploded image changes for now
>  - ... and 2 more: https://git.openjdk.org/jdk/compare/7695b1f9...0e802079

src/java.base/share/native/libjimage/jimage.cpp line 125:

> 123:     if (1 + module_name_len + preview_infix_len + 1 + name_len + 1 > 
> IMAGE_MAX_PATH) {
> 124:         return 0L;
> 125:     }

It would be a bug in the caller to accidentally prepend /modules or /packages 
to the module name. I'm tempted to suggest this assert as it can't assert in 
the caller (at least not without an error code parameter, or having some non-0 
values reserved for errors).

I can't think of how it would be possible to map a package name to the name of 
a module in the image but that name be larger than what is possible. So maybe 
that should assert too.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/29414#discussion_r3001430330

Reply via email to