Hi Xuelei,
I second Alan's suggestion to use VarHandle CAS for getDefault().
Otherwise you will have a race between getDefault() and setDefault()
which will be problematic (and could make tests/app fail randomly).
WRT default factories I like the new holder class.
I wonder if you should bite the bullet and take this opportunity
to get rid of the @SuppressWarnings("deprecation") by replacing
calls to Class::newInstance by calls to Class::getConstructor,
as was done elsewhere in the JDK?
HttpsURLConnectionImpl.java:
If delegate is not final it should now be volatile.
I am not seeing any subclasses of HttpsURLConnectionImpl in JDK 13.
I guess 'plugin' meant 'plugin for applets'?
InputRecord.java
Given that isClosed can only transition from `false` to `true`
you could add a fastpath to isClosed():
if (isClosed) return true;
// otherwise lock and look at the value again
OutputRecord.java and SSLEngineOutputRecord.java:
I was wondering why you didn't use the lock in isClosed(), and
noticed that OutputRecord declares isClosed as volatile.
Which makes me wonder if InputRecord should do the same, and
just forget the lock in InputRecord::isClosed?
Otherwise it looks all reasonable to me.
best regards,
-- daniel
On 03/04/2019 20:33, Xuelei Fan wrote:
On 4/3/2019 12:06 PM, Alan Bateman wrote:
On 03/04/2019 19:40, Xuelei Fan wrote:
Updated webrev (updated SSLContext.java, HttpsClient,
HttpsURLConnectionImpl):
http://cr.openjdk.java.net/~xuelei/8221882/webrev.01/
This looks good to me, the only change that I'm not 100% sure on is in
HttpsClient where the locking will not be consistent with the locking
in the HttpClient super class. Would you mind doubling checking that
one? We do need to change the HTTP protocol handler so I expect
HttpClient will change but it may have to add a protected lock that
HttpsClient uses to ensure that they are using the same lock.
Hm, the update is problematic. Thanks for the catching!
It might be better to make the update together with other http code in
another enhancement. I'd like not update the HttpsClient update this time.
new webrev (remove update for HttpsClient.java):
http://cr.openjdk.java.net/~xuelei/8221882/webrev.02/
Thanks,
Xuelei
