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

[Robert Stupp]

On Sun, 23 Feb 2025 11:32:41 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`).

Should the "sealing checks" be moved from URLClassLoader to ClassLoader?

I've reverted the changes to `[Named]Package.java` and added the "compatible" checks directly to `CL.definePackage()`, using the existing `isSealed` checks.

Unrelated to this change, I wonder whether `java.lang.Package#isSealed(java.net.URL)` should be changed to prevent involving `URLStreamHandler` via `URL.equals`. WDYT?

src/java.base/share/classes/java/lang/NamedPackage.java line 94:

> 92:         return (o instanceof NamedPackage other)
> 93:                 && name.equals(other.name)
> 94:                 && module == other.module;

I'm not sure about the object-equality check here wrt module-layers.

src/java.base/share/classes/java/lang/NamedPackage.java line 99:

> 97:     @Override
> 98:     public int hashCode() {
> 99:         return name.hashCode();

I might be wrong, but IIRC object-identity-hash-code got/gets is treated in a special way w/ Lilliput, so I omitted `module` here.

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

PR Comment: https://git.openjdk.org/jdk/pull/23737#issuecomment-2676805462
PR Comment: https://git.openjdk.org/jdk/pull/23737#issuecomment-2676915966
PR Review Comment: https://git.openjdk.org/jdk/pull/23737#discussion_r1966750832
PR Review Comment: https://git.openjdk.org/jdk/pull/23737#discussion_r1966750746