From 5c57b5de5bbe3c5297d434842acd58e2d61c566d Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Fri, 17 Oct 2014 16:11:40 +0200 Subject: [PATCH] Check for errors without object allocation Motivation: At the moment we use SSL.getLastError() in unwrap(...) to check for error. This is very inefficient as it creates a new String for each check and we also use a String.startsWith(...) to detect if there was an error we need to handle. Modifications: Use SSL.getLastErrorNumber() to detect if we need to handle an error, as this only returns a long and so no String creation happens. Also the detection is much cheaper as we can now only compare longs. Once an error is detected the lately SSL.getErrorString(long) is used to conver the error number to a String and include it in log and exception message. Result: Performance improvements in OpenSslEngine.unwrap(...) due less object allocation and also faster comparations. --- .../main/java/io/netty/handler/ssl/OpenSsl.java | 5 ++--- .../io/netty/handler/ssl/OpenSslClientContext.java | 4 ++-- .../java/io/netty/handler/ssl/OpenSslEngine.java | 14 ++++++++------ .../io/netty/handler/ssl/OpenSslServerContext.java | 13 +++++++++---- 4 files changed, 21 insertions(+), 15 deletions(-) diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSsl.java b/handler/src/main/java/io/netty/handler/ssl/OpenSsl.java index 0036949928..ac06375ac0 100644 --- a/handler/src/main/java/io/netty/handler/ssl/OpenSsl.java +++ b/handler/src/main/java/io/netty/handler/ssl/OpenSsl.java @@ -30,7 +30,6 @@ public final class OpenSsl { private static final InternalLogger logger = InternalLoggerFactory.getInstance(OpenSsl.class); private static final Throwable UNAVAILABILITY_CAUSE; - private static final String IGNORABLE_ERROR_PREFIX = "error:00000000:"; static { Throwable cause = null; @@ -79,8 +78,8 @@ public final class OpenSsl { return UNAVAILABILITY_CAUSE; } - static boolean isError(String error) { - return error != null && !error.startsWith(IGNORABLE_ERROR_PREFIX); + static boolean isError(long errorCode) { + return errorCode != SSL.SSL_ERROR_NONE; } private OpenSsl() { } diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslClientContext.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslClientContext.java index 1f1a8b3152..d325282c20 100644 --- a/handler/src/main/java/io/netty/handler/ssl/OpenSslClientContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslClientContext.java @@ -115,11 +115,11 @@ public final class OpenSslClientContext extends OpenSslContext { if (certChainFile != null) { /* Load the certificate chain. We must skip the first cert when server mode */ if (!SSLContext.setCertificateChainFile(ctx, certChainFile.getPath(), true)) { - String error = SSL.getLastError(); + long error = SSL.getLastErrorNumber(); if (OpenSsl.isError(error)) { throw new SSLException( "failed to set certificate chain: " - + certChainFile + " (" + SSL.getLastError() + ')'); + + certChainFile + " (" + SSL.getErrorString(error) + ')'); } } } diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslEngine.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslEngine.java index d7e2877442..c1cc03a6c7 100644 --- a/handler/src/main/java/io/netty/handler/ssl/OpenSslEngine.java +++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslEngine.java @@ -504,17 +504,18 @@ public final class OpenSslEngine extends SSLEngine { } // Check for OpenSSL errors caused by the priming read - String error = SSL.getLastError(); + long error = SSL.getLastErrorNumber(); if (OpenSsl.isError(error)) { + String err = SSL.getErrorString(error); if (logger.isInfoEnabled()) { logger.info( "SSL_read failed: primingReadResult: " + lastPrimingReadResult + - "; OpenSSL error: '" + error + '\''); + "; OpenSSL error: '" + err + '\''); } // There was an internal error -- shutdown shutdown(); - throw new SSLException(error); + throw new SSLException(err); } // There won't be any application data until we're done handshaking @@ -953,16 +954,17 @@ public final class OpenSslEngine extends SSLEngine { int code = SSL.doHandshake(ssl); if (code <= 0) { // Check for OpenSSL errors caused by the handshake - String error = SSL.getLastError(); + long error = SSL.getLastErrorNumber(); if (OpenSsl.isError(error)) { + String err = SSL.getErrorString(error); if (logger.isInfoEnabled()) { logger.info( - "SSL_do_handshake failed: OpenSSL error: '" + error + '\''); + "SSL_do_handshake failed: OpenSSL error: '" + err + '\''); } // There was an internal error -- shutdown shutdown(); - throw new SSLException(error); + throw new SSLException(err); } } } diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslServerContext.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslServerContext.java index 01bf6a3905..3361508167 100644 --- a/handler/src/main/java/io/netty/handler/ssl/OpenSslServerContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslServerContext.java @@ -189,10 +189,11 @@ public final class OpenSslServerContext extends OpenSslContext { /* Load the certificate chain. We must skip the first cert when server mode */ if (!SSLContext.setCertificateChainFile(ctx, certChainFile.getPath(), true)) { - String error = SSL.getLastError(); + long error = SSL.getLastErrorNumber(); if (OpenSsl.isError(error)) { + String err = SSL.getErrorString(error); throw new SSLException( - "failed to set certificate chain: " + certChainFile + " (" + SSL.getLastError() + ')'); + "failed to set certificate chain: " + certChainFile + " (" + err + ')'); } } @@ -200,8 +201,12 @@ public final class OpenSslServerContext extends OpenSslContext { try { if (!SSLContext.setCertificate( ctx, certChainFile.getPath(), keyFile.getPath(), keyPassword, SSL.SSL_AIDX_RSA)) { - throw new SSLException("failed to set certificate: " + - certChainFile + " and " + keyFile + " (" + SSL.getLastError() + ')'); + long error = SSL.getLastErrorNumber(); + if (OpenSsl.isError(error)) { + String err = SSL.getErrorString(error); + throw new SSLException("failed to set certificate: " + + certChainFile + " and " + keyFile + " (" + err + ')'); + } } } catch (SSLException e) { throw e;