Merge pull request #415 from padelsbach/catch-exceptions

Enhancements for socket error handling
pull/416/merge
Chris Conlon 2026-09-11 15:59:11 -06:00 committed by GitHub
commit 537254ec10
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 225 additions and 62 deletions

View File

@ -2083,7 +2083,15 @@ public class WolfSSLSocket extends SSLSocket {
/* Free only on last op out and close() is pending, so ops that exit
* with others ongoing skip the socket monitor. */
if ((activeOperations.decrementAndGet() == 0) && closeRequested) {
freeSSLIfInactive();
try {
freeSSLIfInactive();
} catch (IllegalStateException | WolfSSLJNIException |
WolfSSLException e) {
/* close() has already returned to the application here,
* so log the failure instead of propagating it. */
WolfSSLDebug.log(getClass(), WolfSSLDebug.ERROR,
() -> "exception freeing this.ssl on I/O exit: " + e);
}
}
}
@ -2094,11 +2102,21 @@ public class WolfSSLSocket extends SSLSocket {
* Takes socket monitor then ioLock (matching close()) to order the free
* against other this.ssl readers and free at most once.
*
* May run on an I/O thread exiting read()/write(): logs a freeSSL() error
* rather than propagating it, and can block briefly on the socket monitor
* behind a concurrent close().
* May run on an I/O thread exiting read()/write(), where it can block
* briefly on the socket monitor behind a concurrent close().
*
* Failures are left to the caller: close() reports them to the
* application as IOException, the I/O exit path has no caller left to
* report to and logs them instead.
*
* @throws IllegalStateException if the native session has already been
* freed
* @throws WolfSSLJNIException if the native free fails
* @throws WolfSSLException if the native free fails, which the JNI layer
* raises in place of WolfSSLJNIException
*/
private void freeSSLIfInactive() {
private void freeSSLIfInactive()
throws IllegalStateException, WolfSSLJNIException, WolfSSLException {
/* No free pending. Free clears the flag, so a closed socket short
* circuits here too. */
@ -2108,47 +2126,40 @@ public class WolfSSLSocket extends SSLSocket {
synchronized (this) {
synchronized (ioLock) {
try {
if (this.ssl == null) {
closeRequested = false;
return;
}
/* Some thread could still be using the session. */
final int polled = this.ssl.getThreadsBlockedInPoll();
final int active = activeOperations.get();
final boolean streamsClosed = ioStreamsAreClosed();
if ((polled != 0) || !streamsClosed || (active != 0)) {
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
() -> "deferring freeing this.ssl, poll: " +
polled + ", streamsClosed: " + streamsClosed +
", active: " + active);
return;
}
/* Close ConsumedRecvCtx data streams before free */
Object readCtx = this.ssl.getIOReadCtx();
if (readCtx instanceof ConsumedRecvCtx) {
try {
((ConsumedRecvCtx)readCtx).closeDataStreams();
} catch (IOException ioe) {
WolfSSLDebug.log(getClass(), WolfSSLDebug.ERROR,
() -> "error closing ConsumedRecvCtx " +
"streams: " + ioe);
}
}
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
() -> "freeing this.ssl from freeSSLIfInactive()");
this.ssl.freeSSL();
this.ssl = null;
if (this.ssl == null) {
closeRequested = false;
} catch (IllegalStateException | WolfSSLJNIException e) {
WolfSSLDebug.log(getClass(), WolfSSLDebug.ERROR,
() -> "exception freeing this.ssl in " +
"freeSSLIfInactive(): " + e);
return;
}
/* Some thread could still be using the session. */
final int polled = this.ssl.getThreadsBlockedInPoll();
final int active = activeOperations.get();
final boolean streamsClosed = ioStreamsAreClosed();
if ((polled != 0) || !streamsClosed || (active != 0)) {
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
() -> "deferring freeing this.ssl, poll: " +
polled + ", streamsClosed: " + streamsClosed +
", active: " + active);
return;
}
/* Close ConsumedRecvCtx data streams before free */
Object readCtx = this.ssl.getIOReadCtx();
if (readCtx instanceof ConsumedRecvCtx) {
try {
((ConsumedRecvCtx)readCtx).closeDataStreams();
} catch (IOException ioe) {
WolfSSLDebug.log(getClass(), WolfSSLDebug.ERROR,
() -> "error closing ConsumedRecvCtx " +
"streams: " + ioe);
}
}
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
() -> "freeing this.ssl from freeSSLIfInactive()");
this.ssl.freeSSL();
this.ssl = null;
closeRequested = false;
}
}
}
@ -2183,8 +2194,10 @@ public class WolfSSLSocket extends SSLSocket {
* If this socket was created with an autoClose value set to true,
* this will also close the underlying Socket.
*
* close() logs a native session free failure rather than throwing, since
* the free may run on a later I/O thread. See freeSSLIfInactive().
* A native session free that fails is reported as IOException once this
* socket is otherwise closed and its transport released. One deferred to
* an I/O thread still running here cannot report back to this caller and
* is logged there instead. See freeSSLIfInactive().
*
* @throws IOException upon error closing the connection
*/
@ -2194,6 +2207,7 @@ public class WolfSSLSocket extends SSLSocket {
int ret;
boolean beforeObjectInit = false;
boolean handshakeFinished = false;
Exception freeException = null;
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
() -> "entered close()");
@ -2355,7 +2369,15 @@ public class WolfSSLSocket extends SSLSocket {
* and the interruptFds[] pipe earlier than finalize(),
* or defer to the last exiting I/O operation if a thread
* could still be using it. */
freeSSLIfInactive();
try {
freeSSLIfInactive();
} catch (IllegalStateException | WolfSSLJNIException |
WolfSSLException e) {
/* Report below, after the rest of close() has run,
* so a failed free still leaves this socket closed
* and its transport released. */
freeException = e;
}
/* Mark closed on every teardown path, before clearing
* EngineHelper, so a later startHandshake() won't NPE.
@ -2401,6 +2423,10 @@ public class WolfSSLSocket extends SSLSocket {
} catch (IllegalStateException e) {
throw new IOException(e);
}
if (freeException != null) {
throw new IOException(freeException);
}
}
/**

View File

@ -56,6 +56,7 @@ import java.net.Socket;
import java.net.ServerSocket;
import java.net.SocketAddress;
import java.net.SocketException;
import java.net.SocketTimeoutException;
import java.net.ConnectException;
import javax.net.SocketFactory;
import javax.net.ssl.SSLSocket;
@ -106,7 +107,10 @@ import com.wolfssl.provider.jsse.WolfSSLProvider;
import com.wolfssl.provider.jsse.WolfSSLSocketFactory;
import com.wolfssl.provider.jsse.WolfSSLSocket;
import com.wolfssl.WolfSSL;
import com.wolfssl.WolfSSLContext;
import com.wolfssl.WolfSSLException;
import com.wolfssl.WolfSSLJNIException;
import com.wolfssl.WolfSSLSession;
/* Tests run by this class:
public void testGetSupportedCipherSuites();
@ -3683,9 +3687,11 @@ public class WolfSSLSocketTest {
return (fds == null) ? -1 : fds.length;
}
/* Bound the setup handshake. On failure tear down the server side (plain
* Socket first, so its blocked handshake read returns) and return the
* failure, else null with SO_TIMEOUT reset. */
/* Bound the setup handshake. Tear down the server side on failure (plain
* Socket first, so its blocked handshake read returns), then return a
* starvation timeout for the caller to skip, or throw anything else since
* only the timeout is expected here. Returns null with SO_TIMEOUT reset
* when the handshake succeeded. */
private static Throwable trySetupHandshake(SSLSocket cs, Socket plain,
SSLServerSocket ss, SSLSocket server, Future<Void> serverFuture)
throws Exception {
@ -3698,6 +3704,9 @@ public class WolfSSLSocketTest {
closeQuietly(ss);
serverFuture.get(30, TimeUnit.SECONDS);
closeQuietly(server);
if (!(e instanceof SocketTimeoutException)) {
throw e;
}
return e;
}
cs.setSoTimeout(0);
@ -3712,6 +3721,7 @@ public class WolfSSLSocketTest {
public void testSocketCloseDuringConcurrentWrite() throws Exception {
int i;
int attempted = 0;
int completed = 0;
Throwable lastSetupExc = null;
String protocol = null;
@ -3804,8 +3814,9 @@ public class WolfSSLSocketTest {
});
/* Busy spinners above can starve this setup handshake
* until it fails or blocks. Bound it and skip the
* iteration on failure or timeout. */
* until it times out. Bound it and skip the iteration
* when that happens. */
attempted++;
Throwable setupExc =
trySetupHandshake(cs, plain, ss, server, serverFuture);
if (setupExc != null) {
@ -3876,9 +3887,12 @@ public class WolfSSLSocketTest {
es.awaitTermination(30, TimeUnit.SECONDS);
}
/* Guard against a silent pass if every setup handshake was skipped */
assertTrue("no iteration completed its setup handshake, last: " +
lastSetupExc, completed > 0);
/* Guard against a silent pass: the race is only exercised by the
* iterations that got past their setup handshake, so most of them
* must have completed one */
assertTrue("only " + completed + " of " + attempted + " iterations " +
"completed their setup handshake, last: " + lastSetupExc,
(completed > 0) && (completed >= ((attempted + 1) / 2)));
}
/* Races close() against InputStream.read() under CPU load, verifying
@ -3890,6 +3904,7 @@ public class WolfSSLSocketTest {
public void testSocketCloseDuringConcurrentRead() throws Exception {
int i;
int attempted = 0;
int completed = 0;
Throwable lastSetupExc = null;
String protocol = null;
@ -3984,8 +3999,9 @@ public class WolfSSLSocketTest {
});
/* Busy spinners above can starve this setup handshake
* until it fails or blocks. Bound it and skip the
* iteration on failure or timeout. */
* until it times out. Bound it and skip the iteration
* when that happens. */
attempted++;
Throwable setupExc =
trySetupHandshake(cs, plain, ss, server, serverFuture);
if (setupExc != null) {
@ -4059,9 +4075,12 @@ public class WolfSSLSocketTest {
es.awaitTermination(30, TimeUnit.SECONDS);
}
/* Guard against a silent pass if every setup handshake was skipped */
assertTrue("no iteration completed its setup handshake, last: " +
lastSetupExc, completed > 0);
/* Guard against a silent pass: the race is only exercised by the
* iterations that got past their setup handshake, so most of them
* must have completed one */
assertTrue("only " + completed + " of " + attempted + " iterations " +
"completed their setup handshake, last: " + lastSetupExc,
(completed > 0) && (completed >= ((attempted + 1) / 2)));
}
/* Closing an SSLSocket mid-write makes close() defer the native
@ -4322,8 +4341,8 @@ public class WolfSSLSocketTest {
this.ctx = tf.createSSLContext(protocol, ctxProvider);
ExecutorService es = Executors.newCachedThreadPool();
final int iterations = 50;
final int warmupIterations = 5;
final int iterations = 200;
final int warmupIterations = 10;
long baseline = -1;
/* Retain closed sockets so finalize() cannot free a leaked pipe and
* mask a reverted build. */
@ -4435,6 +4454,9 @@ public class WolfSSLSocketTest {
}
}
/* A deferred free that never runs leaks the 2-descriptor
* interrupt pipe every measured iteration, several times the
* bound below, leaving the rest as headroom for unrelated fds. */
assertTrue("fd baseline was never recorded", baseline >= 0);
long after = countOpenFds();
assertTrue("could not count open fds", after >= 0);
@ -4449,6 +4471,121 @@ public class WolfSSLSocketTest {
}
}
/* WolfSSLSession whose native free fails, so close() has a session
* cleanup failure to report. */
private static class FreeFailSession extends WolfSSLSession {
private final Throwable failure;
FreeFailSession(WolfSSLContext ctx, Throwable failure)
throws WolfSSLException {
super(ctx);
this.failure = failure;
}
@Override
public void freeSSL() {
throwUndeclared(this.failure);
}
/* Free the real session behind this object, so a test that never
* reaches freeSSL() does not leak it. */
void freeSSLForReal() throws WolfSSLJNIException {
super.freeSSL();
}
}
/* Throw a checked exception that the method does not declare, which is
* how the JNI layer raises WolfSSLException out of freeSSL(). */
@SuppressWarnings("unchecked")
private static <E extends Throwable> void throwUndeclared(Throwable t)
throws E {
throw (E)t;
}
/* Fail the native session free during close() and check the failure
* reaches the caller as IOException, after close() has finished closing
* this socket and its underlying transport. */
private void checkCloseReportsFreeFailure(Throwable failure)
throws Exception {
ServerSocket ss = null;
Socket plain = null;
SSLSocket cs = null;
WolfSSLContext jniCtx = null;
FreeFailSession failSsl = null;
WolfSSLSession origSsl = null;
try {
ss = new ServerSocket(0);
plain = new Socket();
plain.connect(new InetSocketAddress("127.0.0.1",
ss.getLocalPort()));
/* autoClose true, so close() owns the underlying Socket */
cs = (SSLSocket)this.ctx.getSocketFactory().createSocket(
plain, "127.0.0.1", ss.getLocalPort(), true);
jniCtx = new WolfSSLContext(WolfSSL.SSLv23_ClientMethod());
failSsl = new FreeFailSession(jniCtx, failure);
Field sslField = WolfSSLSocket.class.getDeclaredField("ssl");
sslField.setAccessible(true);
origSsl = (WolfSSLSession)sslField.get(cs);
sslField.set(cs, failSsl);
try {
cs.close();
fail("close() did not report the native free failure");
} catch (IOException e) {
assertEquals("close() reported the wrong failure",
failure, e.getCause());
}
assertTrue("close() left the underlying Socket open",
cs.isClosed());
/* Drop the injected session, this socket is done with it */
sslField.set(cs, null);
}
finally {
if (failSsl != null) {
failSsl.freeSSLForReal();
}
if (origSsl != null) {
origSsl.freeSSL();
}
if (jniCtx != null) {
jniCtx.free();
}
closeQuietly(plain);
closeQuietly(ss);
}
}
/* A native session free that fails must not be swallowed by close() */
@Test
public void testCloseReportsNativeFreeFailure() throws Exception {
String protocol = null;
if (WolfSSL.TLSv12Enabled()) {
protocol = "TLSv1.2";
} else if (WolfSSL.TLSv13Enabled()) {
protocol = "TLSv1.3";
}
Assume.assumeNotNull(protocol);
this.ctx = tf.createSSLContext(protocol, ctxProvider);
/* freeSSL() declares WolfSSLJNIException, the JNI layer raises
* WolfSSLException, close() has to report either one */
checkCloseReportsFreeFailure(
new WolfSSLJNIException("simulated native free failure"));
checkCloseReportsFreeFailure(
new WolfSSLException("simulated native free failure"));
}
@Test
public void testSocketMethodsAfterClose() throws Exception {