On Mon, 5 Dec 2022 14:06:10 GMT, Daniel Jeliński <djelin...@openjdk.org> wrote:
>> Please review this patch that removes progress monitoring classes used by >> UrlConnection. >> Since Java 9 these classes are not used in the JDK, and are not exported >> from java.base. If anyone was still using them, reimplementing them in user >> code should be pretty straightforward. >> >> This PR also fixes the issue where MeteredStream finalizer could resurrect >> an unusable connection, causing unexpected exceptions in other parts of the >> code. >> >> No new regression test; since we are removing the finalizer, I don't believe >> we will see the issue again. I can add a test for that if you think it still >> makes sense. >> >> I had to adjust `ProxyModuleMapping.java` test which used >> `sun.net.ProgressListener` as an example of a module-private interface; I >> replaced it with another public interface from the same package. >> >> Existing tier 1-3 tests continue to pass. > > Daniel Jeliński has updated the pull request incrementally with one > additional commit since the last revision: > > Add test test/jdk/sun/net/www/http/KeepAliveStream/KeepAliveStreamFinalizer.java line 28: > 26: * @bug 8240275 > 27: * @library /test/lib > 28: * @run main/othervm KeepAliveStreamFinalizer Hello Daniel, it doesn't look like we are doing anything JVM specific in this test. Is the `othervm` intentional here? test/jdk/sun/net/www/http/KeepAliveStream/KeepAliveStreamFinalizer.java line 175: > 173: public InputStream getInputStream() throws IOException { > 174: if (finalized) { > 175: System.out.println(failureReason = "getInputStream > called after finalize"); For ease of debugging, perhaps use `System.err.println` instead of `System.out`? That way the `Thread.dumpStack()` which is done on the next line will appear in the same jtreg section as this message. Same comment in few other places where we have this construct. ------------- PR: https://git.openjdk.org/jdk/pull/11474