[tinkerpop] branch master updated (ff50136 -> 617808b)

[sp...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

spmallette pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git.


    from ff50136  Merge branch '3.4-dev'
     add fa812b8  Removed synchronized from shutdown() in Connection CTR
     new 617808b  Merge branch '3.4-dev'

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../main/java/org/apache/tinkerpop/gremlin/driver/Connection.java  | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

[tinkerpop] 01/01: Merge branch '3.4-dev'

[sp...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

spmallette pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git

commit 617808b6ef659545d762a7f4216ef21e7d13f8f7
Merge: ff50136 fa812b8
Author: Stephen Mallette <st...@amazon.com>
AuthorDate: Fri Apr 23 15:29:10 2021 -0400

    Merge branch '3.4-dev'

 .../main/java/org/apache/tinkerpop/gremlin/driver/Connection.java  | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --cc gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Connection.java
index 7babdf1,143b6ff..b25045f
--- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Connection.java
+++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Connection.java
@@@ -292,8 -344,47 +292,13 @@@ final class Connection 
          // shutdown can be called directly from closeAsync() or after write() and therefore this method should only
          // be called once. once shutdown is initiated, it shouldn't be executed a second time or else it sends more
          // messages at the server and leads to ugly log messages over there.
+         //
+         // this method used to be synchronized prior to 3.4.11, but it seems to be able to leave the driver in a
+         // hanging state because of the write() down below that can call back in on shutdown() (which is weird i
+         // guess). that seems to put the executor thread in a monitor state that it doesn't recover from. since all
+         // the code in here is behind shutdownInitiated the synchronized doesn't seem necessary
          if (shutdownInitiated.compareAndSet(false, true)) {
              final String connectionInfo = this.getConnectionInfo();
 -            // this block of code that "closes" the session is deprecated as of 3.3.11 - this message is going to be
 -            // removed at 3.5.0. we will instead bind session closing to the close of the channel itself and not have
 -            // this secondary operation here which really only acts as a means for clearing resources in a functioning
 -            // session. "functioning" in this context means that the session is not locked up with a long running
 -            // operation which will delay this close execution which ideally should be more immediate, as in the user
 -            // is annoyed that a long run operation is happening and they want an immediate cancellation. that's the
 -            // most likely use case. we also get the nice benefit that this if/then code just goes away as the
 -            // Connection really shouldn't care about the specific Client implementation.
 -            if (client instanceof Client.SessionedClient && !isDead()) {
 -                final boolean forceClose = client.getSettings().getSession().get().isForceClosed();
 -                final RequestMessage closeMessage = client.buildMessage(
 -                        RequestMessage.build(Tokens.OPS_CLOSE).addArg(Tokens.ARGS_FORCE, forceClose)).create();
 -
 -                final CompletableFuture<ResultSet> closed = new CompletableFuture<>();
 -                write(closeMessage, closed);
 -
 -                try {
 -                    // make sure we get a response here to validate that things closed as expected.  on error, we'll let
 -                    // the server try to clean up on its own.  the primary error here should probably be related to
 -                    // protocol issues which should not be something a user has to fuss with.
 -                    closed.join().all().get(cluster.getMaxWaitForSessionClose(), TimeUnit.MILLISECONDS);
 -                } catch (TimeoutException ex) {
 -                    final String msg = String.format(
 -                            "Timeout while trying to close connection on %s - force closing - server will close session on shutdown or expiration.",
 -                            ((Client.SessionedClient) client).getSessionId());
 -                    logger.warn(msg, ex);
 -                } catch (Exception ex) {
 -                    final String msg = String.format(
 -                            "Encountered an error trying to close connection on %s - force closing - server will close session on shutdown or expiration.",
 -                            ((Client.SessionedClient) client).getSessionId());
 -                    logger.warn(msg, ex);
 -                }
 -            }
 -
              channelizer.close(channel);
  
              final ChannelPromise promise = channel.newPromise();