Hi Xuelei,
I am not an expert of the field so please get another
reviewer for this change:
656 if (!super.isOutputShutdown() &&
...
661 } else if (!super.isOutputShutdown()) {
I think it might be clearer with a nested if:
if (!super.isOutputShutdown()) {
if (isLayered() && !autoClose) {
throw exception ...
} else {
super.shutdownOutput();
log ...
}
}
Could lines 630 .. 647 be moved to a private method?
Something like:
private void handleClosedNotifyAlert() {
630 try {
// send a user_canceled alert if needed.
if (useUserCanceled) {
conContext.warning(Alert.USER_CANCELED);
}
// send a close_notify alert
conContext.warning(Alert.CLOSE_NOTIFY);
} finally {
if (!conContext.isOutboundClosed()) {
conContext.outputRecord.close();
}
if (!super.isOutputShutdown() &&
(autoClose || !isLayered())) {
super.shutdownOutput();
647 }
}
}
because if I'm not mistaken this exact same piece of code
appears at lines 700 - 717.
I believe this would simplify this long method a little bit,
and possibly makes what is happening here clearer:
623 if (linger >= 0) {
// don't wait more than SO_LINGER for obtaining the
// the lock...
...
626 try {
627 if (conContext.outputRecord.recordLock.tryLock() ||
628 conContext.outputRecord.recordLock.tryLock(
629 linger, TimeUnit.SECONDS)) {
try {
handleClosedNotifyAlert();
} finally {
conContext.outputRecord.recordLock.unlock();
}
652 } else {
653 // For layered, non-autoclose sockets, we are not
654 // able to bring them into a usable state, so we
655 // treat it as fatal error.
...
688 }
689 } catch (InterruptedException ex) {
...
692 }
693
694 // restore the interrupted status
...
698 } else {
699 conContext.outputRecord.recordLock.lock();
700 try {
handleClosedNotifyAlert();
} finally {
conContext.outputRecord.recordLock.unlock();
}
}
...
BlockedAsyncClose.java:
Can you bind the server to the loopback instead of the
wildcard? My experience is that it make tests more reliable.
InetAddress loopback = InetAddress.getLoopbackAddress();
74 ss = (SSLServerSocket) sslssf.createServerSocket();
ss.bind(new InetSosketAddress(loopback, 0));
75
76 SSLSocketFactory sslsf =
77 (SSLSocketFactory)SSLSocketFactory.getDefault();
socket = (SSLSocket)sslsf.createSocket(loopback,
ss.getLocalPort());
best regards,
-- daniel
On 12/06/2019 14:51, Xuelei Fan wrote:
ping ...
On 6/4/2019 11:10 AM, Xuelei Fan wrote:
Hi,
Could I get the following update reviewed?
http://cr.openjdk.java.net/~xuelei/8224829/webrev.00/
If using one thread for closing, one thread for writing. Closing and
writing threads are synchronized in order to delivery close_notify TLS
record. There are could be race between the two threads. If the
output stream buffer reach the limit, the write will be blocked, and
then the close thread may be pending.
The SO_LINGER is used to resolve the issue for general socket. With
the above update, the SSLSocket implementation will check the
SO_LINGER as well. If SO_LINGER is on, the SSLSocket close will try
to get the synchronization lock in the time of SO_LINGER value. If
timeout, the close process will move on, and close the underlying
socket by force.
Thanks,
Xuelei
