[jira] [Commented] (GEODE-9819) Client socket leak in CacheClientNotifier.registerClientInternal when error conditions occur for the durable client

["ASF subversion and git services (Jira)" <ji...@apache.org>]

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

ASF subversion and git services commented on GEODE-9819:
--------------------------------------------------------

Commit 97601eb2cd585f844b7f02bb73ba42fcb86a7cb4 in geode's branch refs/heads/develop from Darrel Schneider
[ https://gitbox.apache.org/repos/asf?p=geode.git;h=97601eb ]

GEODE-9819: fix durable client socket leak

Added unit test that reproduced the socket leak.
This involved some change to the product classes
to make them unit testable.
Fixed the leak by making sure socket.close is called
if the response code was not successful.

> Client socket leak in CacheClientNotifier.registerClientInternal when error conditions occur for the durable client
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: GEODE-9819
>                 URL: https://issues.apache.org/jira/browse/GEODE-9819
>             Project: Geode
>          Issue Type: Bug
>          Components: client/server, core
>    Affects Versions: 1.12.5, 1.13.4, 1.14.0, 1.15.0
>            Reporter: Leon Finker
>            Assignee: Darrel Schneider
>            Priority: Critical
>              Labels: GeodeOperationAPI, blocks-1.15.0​, pull-request-available
>             Fix For: 1.15.0
>
>
> In CacheClientNotifier.registerClientInternal client socket can be left half open and not properly closed when error conditions occur. Such as the case of:
> {code:java}
> } else {
>   // The existing proxy is already running (which means that another
>   // client is already using this durable id.
>   unsuccessfulMsg =
>       String.format(
>           "The requested durable client has the same identifier ( %s ) as an existing durable client ( %s ). Duplicate durable clients are not allowed.",
>           clientProxyMembershipID.getDurableId(), cacheClientProxy);
>   logger.warn(unsuccessfulMsg);
>   // Set the unsuccessful response byte.
>   responseByte = Handshake.REPLY_EXCEPTION_DUPLICATE_DURABLE_CLIENT;
> } {code}
> It considers the current client connect attempt to have failed. It writes this response back to client: REPLY_EXCEPTION_DUPLICATE_DURABLE_CLIENT. This will cause the client to throw ServerRefusedConnectionException. What seems wrong about this method is that even though it sets "unsuccessfulMsg" and correctly sends back a handshake saying the client is rejected, it does not throw an exception and it does not close "socket". I think right before it calls performPostAuthorization it should do the followiing:
> {code:java}
> if (unsuccessfulMsg != null) {
>       try {
>         socket.close();
>       } catch (IOException ignore) {
>       }
>  } else {
>     performPostAuthorization(...)
> }{code}
> Full discussion details can be found at https://markmail.org/thread/2gqmbq2m57pz7pxu



--
This message was sent by Atlassian Jira
(v8.20.1#820001)