On Wed, 28 Jun 2023 11:15:53 GMT, Jaikiran Pai <[email protected]> wrote:
> Can I please get a review for this change which proposes to address > https://bugs.openjdk.org/browse/JDK-8311032? > > The commit in this PR skips the class lookups when the package name prefix is > empty, either due to the `java.protocol.handler.pkgs` system property value > being empty or due to individual parts of the `|` delimited value being empty. > > This avoids potentially expensive classloading attempts for a class name of > the form `.foo.Handler`, which is bound to fail. > > No new jtreg test has been added given the nature of the change. There's > already an existing test > `test/jdk/java/net/URL/HandlersPkgPrefix/HandlersPkgPrefix.java` which tests > different values for the `java.protocol.handler.pkgs` system property > (including empty value) and that test continues to pass with this change. > > tier testing is currently in progress. Hello Alan, this issue with empty value was noticed when trying to get past the other issue noted in https://bugs.openjdk.org/browse/JDK-8311032. The reporter of that other issue tried to workaround that StackOverflowError, by trying to conditionally set the value for this system property only when they thought it might be necessary. The workaround was of the form '-Djava.protocol.handler.pkg=${var}'. In such builds, 'var' would evaluate to empty when they didn't want to set any genuine package name. It was expected that using this workaround would prevent the StackOverflowError when large classpaths were involved. But given the way this code currently deals with empty value, it still ended up scanning the large classpath for an unnecessary class and eventually again ended up with the StackOverflowError. So this change is to prevent such cases, which in reality should be rare. ------------- PR Comment: https://git.openjdk.org/jdk/pull/14693#issuecomment-1611362069
