[ 
https://issues.apache.org/jira/browse/QPID-8545?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17378797#comment-17378797
 ] 

ASF GitHub Bot commented on QPID-8545:
--------------------------------------

alex-rufous commented on a change in pull request #98:
URL: https://github.com/apache/qpid-broker-j/pull/98#discussion_r667539275



##########
File path: 
broker-core/src/main/java/org/apache/qpid/server/transport/NonBlockingConnectionTLSDelegate.java
##########
@@ -82,6 +86,9 @@ public NonBlockingConnectionTLSDelegate(NonBlockingConnection 
parent, AmqpPort p
         _applicationBuffer = QpidByteBuffer.allocateDirect(_networkBufferSize);
         _netOutputBuffer = QpidByteBuffer.allocateDirect(_networkBufferSize);
         _ignoreInvalidSni = port.isIgnoreInvalidSni();
+        enableDiagnosisOfSslEngineLooping = 
port.getContextValue(Boolean.class, 
AmqpPort.PORT_DIAGNOSIS_OF_SSL_ENGINE_LOOPING);

Review comment:
       A class field should start with underscore character as per Qpid code 
style. Please add underscore...

##########
File path: 
broker-core/src/main/java/org/apache/qpid/server/transport/NonBlockingConnectionTLSDelegate.java
##########
@@ -82,6 +86,9 @@ public NonBlockingConnectionTLSDelegate(NonBlockingConnection 
parent, AmqpPort p
         _applicationBuffer = QpidByteBuffer.allocateDirect(_networkBufferSize);
         _netOutputBuffer = QpidByteBuffer.allocateDirect(_networkBufferSize);
         _ignoreInvalidSni = port.isIgnoreInvalidSni();
+        enableDiagnosisOfSslEngineLooping = 
port.getContextValue(Boolean.class, 
AmqpPort.PORT_DIAGNOSIS_OF_SSL_ENGINE_LOOPING);
+        diagnosisOfSslEngineLoopingWarnThreshold = 
port.getContextValue(Integer.class, 
AmqpPort.PORT_DIAGNOSIS_OF_SSL_ENGINE_LOOPING_WARN_THRESHOLD);
+        diagnosisOfSslEngineLoopingBreakThreshold = 
port.getContextValue(Integer.class, 
AmqpPort.PORT_DIAGNOSIS_OF_SSL_ENGINE_LOOPING_BREAK_THRESHOLD);

Review comment:
       A class field should start with underscore character as per Qpid code 
style. Please add underscore...

##########
File path: 
broker-core/src/main/java/org/apache/qpid/server/transport/NonBlockingConnection.java
##########
@@ -34,6 +34,7 @@
 import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;

Review comment:
       unused import

##########
File path: 
broker-core/src/main/java/org/apache/qpid/server/transport/NonBlockingConnectionTLSDelegate.java
##########
@@ -310,6 +337,11 @@ private int wrapBufferArray(Collection<QpidByteBuffer> 
buffers) throws SSLExcept
         }
         while(encrypted && _sslEngine.getHandshakeStatus() != 
SSLEngineResult.HandshakeStatus.NEED_UNWRAP);
 
+        if (enableDiagnosisOfSslEngineLooping)

Review comment:
       Daniil,
   Thanks for adding  a reset for a loop counter...
   Though, I am wondering whether the counter reset should be made 
conditional... 
   As far as I remember issues reported as part of QPID-8545 and QPID-8526, the 
connection was looping in `NonBlockingConnection`, 
`NonBlockingConnectionTLSDelegate`, etc
   Thus, it seems that counter should be reset only if data was actually 
written into the output buffer... Otherwise, you might not detect an engine  
issue when encryption loop ends without writing any data...
   What do you think?
   

##########
File path: 
broker-core/src/main/java/org/apache/qpid/server/transport/NonBlockingConnectionTLSDelegate.java
##########
@@ -82,6 +86,9 @@ public NonBlockingConnectionTLSDelegate(NonBlockingConnection 
parent, AmqpPort p
         _applicationBuffer = QpidByteBuffer.allocateDirect(_networkBufferSize);
         _netOutputBuffer = QpidByteBuffer.allocateDirect(_networkBufferSize);
         _ignoreInvalidSni = port.isIgnoreInvalidSni();
+        enableDiagnosisOfSslEngineLooping = 
port.getContextValue(Boolean.class, 
AmqpPort.PORT_DIAGNOSIS_OF_SSL_ENGINE_LOOPING);
+        diagnosisOfSslEngineLoopingWarnThreshold = 
port.getContextValue(Integer.class, 
AmqpPort.PORT_DIAGNOSIS_OF_SSL_ENGINE_LOOPING_WARN_THRESHOLD);

Review comment:
       A class field should start with underscore character as per Qpid code 
style. Please add underscore...




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


> [Broker-J] SSL Engine looping circuit breaker
> ---------------------------------------------
>
>                 Key: QPID-8545
>                 URL: https://issues.apache.org/jira/browse/QPID-8545
>             Project: Qpid
>          Issue Type: Improvement
>          Components: Broker-J
>    Affects Versions: qpid-java-broker-8.0.5
>            Reporter: Daniil Kirilyuk
>            Priority: Minor
>
> A configurable circuit breaker should be added to 
> NonBlockingConnectionTLSDelegate giving the possibility to stop SSL engine 
> looping (QPID-8526, QPID-8489).



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to