RFR: 8349545: ClassLoader.definePackage() throws IllegalArgumentException if package already defined [v2]

[Alan Bateman]

On Mon, 24 Mar 2025 07:53:26 GMT, Robert Stupp <rstupp at openjdk.org> wrote:

>> Concurent calls to `ClassLoader.definePackage()` can yield `IllegalArgumentException`s if the package is already defined. Some built-in class loaders, like `URLClassLoader`, already handle this case, but custom class loaders (would) have to handle this case.
>> 
>> This change updates the logic of `CL.definePackage` to not throw an IAE if the "redefined" package is equal to the already defined one.
>> 
>> Tests added in `jdk/java/lang/Package/PackageDefineTest.java` (+ pulling up the `TestClassLoader` from `PackageDefineTest`).
>
> Robert Stupp has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
> 
>  - spec (javadoc)
>  - move checks to CL + hoist IAE checks from URL/BuiltinClassLoaders
>  - 8349545: ClassLoader.definePackage() throws IllegalArgumentException if package already defined
>    
>    Concurent calls to `ClassLoader.definePackage()` can yield `IllegalArgumentException`s if the package is already defined. Some built-in class loaders, like `URLClassLoader`, already handle this case, but custom class loaders (would) have to handle this case.
>    
>    This change updates the logic of `CL.definePackage` to not throw an IAE if the "redefined" package is equal to the already defined one. To enable this, `NamedPackage`, `Package` and `Package$VersionInfo` implement `hashCode`+`equals`, with the special handling to not involve a `URLStreamHandler` when comparing `VersionInfo.sealBase`.
>    
>    Tests added in `jdk/java/lang/Package/PackageDefineTest.java` (+ pulling up the `TestClassLoader` from `PackageDefineTest`).

Just to say again, I don't think the proposal in this PR can go forward. It changes this 25+ year old API in a significant way. A new API could be explored but I think first needs looking at custom class loader implementations to get a sense as to whether it would be used or not.

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

PR Comment: https://git.openjdk.org/jdk/pull/23737#issuecomment-2747218529