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

ASF GitHub Bot commented on PROTON-2275:
----------------------------------------

astitcher commented on code in PR #255:
URL: https://github.com/apache/qpid-proton/pull/255#discussion_r1295135521


##########
c/src/ssl/schannel.cpp:
##########
@@ -1765,6 +1765,12 @@ static ssize_t process_output_ssl( pn_transport_t 
*transport, unsigned int layer
   if (!ssl) return PN_EOS;
   ssl_log( transport, PN_LEVEL_TRACE, "process_output_ssl( max_len=%d 
)",max_len );
 
+  // This is to match the behaviour of pn_output_write_amqp defined in 
transport.c.
+  // Without this, the idle_timeout connection option does not work in case of 
an SSL connection.
+  if (!pn_buffer_size(transport->output_buffer) && transport->close_sent) {
+    return PN_EOS;
+  }
+

Review Comment:
   I'm somewhat confused about this change - the equivalent openssl code does 
not do anything like this but as far as I know works correctly in the face of 
idle timeout. This makes me think that this is not the correct fix to the 
problem.
   Especially as the check should be redundent in that the amqp layer 
processing which does have this check will still be running and should pick up 
on the issue. Clearly there is an issue somewhere that is specific to the 
schannel implementation, but this seems like a 'voodoo' fix.
   @cliffjansen @kgiusti do either of you have any thoughts?





> idle_timeout connection option not working for SSL connections on Windows
> -------------------------------------------------------------------------
>
>                 Key: PROTON-2275
>                 URL: https://issues.apache.org/jira/browse/PROTON-2275
>             Project: Qpid Proton
>          Issue Type: Bug
>          Components: proton-c
>         Environment: Windows client connecting to SSL broker
>            Reporter: Attila Kun
>            Priority: Major
>
> Steps to reproduce:
>  # Connect your receiver to an SSL-capable broker. The receiver has to be 
> running on Windows.
>  # Set the {{idle_timeout}} connection option to e.g. 2 seconds in your 
> {{proton::messaging_handler}} instance.
>  # In the {{on_message}} callback, sleep for longer than {{idle_timeout}}. 
> This should trigger the {{on_transport_error}} callback with a 
> {{amqp:resource-limit-exceeded: local-idle-timeout}} expired error. On 
> Windows this does not happen.
> I saw that the above scenario worked correctly when connecting to a non-SSL 
> broker but failed with an SSL broker. I looked at the difference between the 
> two code paths and found that for the same input 
> ({{transport->output_buffer}} being empty and {{transport->close_sent == 
> true}}) [transport.c returns 
> PN_EOS|https://github.com/apache/qpid-proton/blob/0.31.0/c/src/core/transport.c#L2722]
>  while [schannel.c returned 
> 0|https://github.com/apache/qpid-proton/blob/0.31.0/c/src/ssl/schannel.c#L1850].
>  After ensuring that schannel.c matches the behaviour of transport.c, the 
> problem went away.
> Suggested fix: https://github.com/apache/qpid-proton/pull/255



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org

Reply via email to