Hi,
This is a fix that upgrades the old HTTP and HTTPS legacy stack to use
virtual-thread friendly locking instead of
synchronized monitors.
Most of the changes are mechanical - but there are still a numbers of subtle
non-mechanical differences that are
outlined below:
1. src/java.base/share/classes/sun/net/www/MessageHeader.java:
`MessageHeader::print(PrintStream)` => synchronization modified to not
synchronize on this while printing
( a snapshot of the data is taken before printing instead)
2. src/java.base/share/classes/sun/net/www/MeteredStream.java:
`MeteredStream::close` was missing synchronization: it is now protected by
the lock
3. src/java.base/share/classes/sun/net/www/http/ChunkedOutputStream.java:
`ChunkedOutputStream` no longer extends `PrintStream` but extends
`OutputStream` directly.
Extending `PrintStream` is problematic for virtual thread. After careful
analysis, it appeared that
`ChunkedOutputStream` didn't really need to extend `PrintStream`.
`ChunkedOutputStream`
is already wrapping a `PrintStream`. `ChunkedOutputStream` is never
returned directly to user
code but is wrapped in another stream. `ChunkedOutputStream` completely
reimplement and
reverse the flush logic implemented by its old super class`PrintStream`
which leads me to believe
there was no real reason for it to extend `PrintStream` - except for being
able to call its `checkError`
method - which can be done by using `instanceof ChunkedOutputStream` in the
caller instead of
casting to PrintStream.
4. src/java.base/share/classes/sun/net/www/http/HttpClient.java:
Synchronization removed from `HttpClient::privilegedOpenServer` and
replaced by an `assert`.
There is no need for a synchronized here as the method is only called from
a method that already
holds the lock.
5. src/java.base/share/classes/sun/net/www/http/KeepAliveCache.java:
Synchronization removed from `KeepAliveCache::removeVector` and replaced by
an `assert`.
This method is only called from methods already protected by the lock.
Also the method has been made private.
6. src/java.base/share/classes/sun/net/www/http/KeepAliveStream.java
`queuedForCleanup` is made volatile as it is read and written directly from
outside the class
and without protection by the `KeepAliveCleanerEntry`.
Lock protection is also added to `close()`, which was missing it.
Some methods that have no lock protection and did not need it because only
called from
within code blocks protected by the lock have aquired an `assert
isLockHeldByCurrentThread();`
7. Concrete subclasses of `AuthenticationInfo` that provide an implementation
for
`AuthenticationInfo::setHeaders(HttpURLConnection conn, HeaderParser p,
String raw)` have
acquired an `assert conn.isLockheldByCurrentThread();` as the method should
only be called
from within a lock-protected block in `s.n.w.p.h.HttpURLConnection`
8. src/java.base/share/classes/sun/net/www/protocol/http/HttpURLConnection.java
Several methods in this class have a acquired an `assert
isLockheldByCurrentThread();`
to help track the fact that AuthenticationInfo::setHeaders is only called
while
holding the lock.
Synchronization was also removed from some method that didn't need it
because only
called from within code blocks protected by the lock:
`getOutputStream0()`: synchronization removed and replaced by an `assert
isLockheldByCurrentThread();`
`setCookieHeader()`: synchronization removed and replaced by an `assert
isLockheldByCurrentThread();`
`getInputStream0()`: synchronization removed and replaced by an `assert
isLockheldByCurrentThread();`
`StreamingOutputStream`: small change to accomodate point 3. above (changes
in ChunkedOutputStream).
9.
src/java.base/share/classes/sun/net/www/protocol/http/NegotiateAuthentication.java.:
synchronization removed from `setHeaders` and replace by an assert. See
point 7. above.
10.
src/java.base/unix/classes/sun/net/www/protocol/http/ntlm/NTLMAuthentication.java:
synchronization removed from `setHeaders` and replace by an assert. See
point 7. above.
11.
src/java.base/windows/classes/sun/net/www/protocol/http/ntlm/NTLMAuthentication.java:
synchronization removed from `setHeaders` and replace by an assert. See
point 7. above.
-------------
Commit messages:
- 8229867: Re-examine synchronization usages in http and https protocol
handlers
Changes: https://git.openjdk.java.net/jdk/pull/558/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=558&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8229867
Stats: 1116 lines in 18 files changed: 551 ins; 149 del; 416 mod
Patch: https://git.openjdk.java.net/jdk/pull/558.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/558/head:pull/558
PR: https://git.openjdk.java.net/jdk/pull/558
