[geode] 03/03: * GEODE-8652: NioSslEngine.close() Bypasses Locks (#5712)

2020-11-06 Thread burcham
This is an automated email from the ASF dual-hosted git repository.

burcham pushed a commit to branch support/1.13
in repository https://gitbox.apache.org/repos/asf/geode.git

commit 32a8a55df4c0a5b8b1e5cfb746070804cbce5e0a
Author: Bill Burcham 
AuthorDate: Thu Nov 5 17:30:29 2020 -0800

* GEODE-8652: NioSslEngine.close() Bypasses Locks (#5712)

- NioSslEngine.close() proceeds even if readers (or writers) are
  operating on its ByteBuffers, allowing Connection.close() to close
  its socket and proceed.

- NioSslEngine.close() needed a lock only on the output buffer, so
  we split what was a single lock into two. Also instead of using
  synchronized we use a ReentrantLock so we can
  call tryLock() and time out if needed in NioSslEngine.close().

- Since readers/writers may hold locks on these input/output buffers
  when NioSslEngine.close() is called a reference count is maintained
  and the buffers are returned to the pool only when the last user
  is done.

- To manage the locking and reference counting a new AutoCloseable
  ByteBufferSharing interface is introduced with a trivial
  implementation: ByteBufferSharingNoOp and a real implementation:
  ByteBufferSharingImpl.

- Added a new unit test, and a new concurrency test for
  ByteBufferSharingImpl: both ensure that ByteBuffers are returned
  to the pool exactly once. Added a new DUnit test for the interaction
  between ByteBufferSharingImpl and NioSslEngine and Connection.

Co-authored-by: Bill Burcham 
Co-authored-by: Darrel Schneider 
Co-authored-by: Ernie Burghardt 
Co-authored-by: Dan Smith 
(cherry picked from commit af267c005a63317cbb8528cdb38eccf6a8747818)
---
 .../tcp/ConnectionCloseSSLTLSDUnitTest.java| 238 
 .../org/apache/geode/internal/tcp/server.keystore  | Bin 0 -> 1256 bytes
 ...LSocketHostNameVerificationIntegrationTest.java |   4 +-
 .../internal/net/SSLSocketIntegrationTest.java |  57 ++-
 .../apache/geode/codeAnalysis/excludedClasses.txt  |   1 +
 .../geode/internal/net/ByteBufferSharing.java  |  55 +++
 .../geode/internal/net/ByteBufferSharingImpl.java  | 168 
 .../geode/internal/net/ByteBufferSharingNoOp.java  |  52 +++
 .../org/apache/geode/internal/net/NioFilter.java   |  69 ++--
 .../apache/geode/internal/net/NioPlainEngine.java  |  27 +-
 .../apache/geode/internal/net/NioSslEngine.java| 365 -
 .../org/apache/geode/internal/tcp/Connection.java  |  34 +-
 .../org/apache/geode/internal/tcp/MsgReader.java   |  15 +-
 .../internal/net/ByteBufferConcurrencyTest.java| 165 
 .../internal/net/ByteBufferSharingImplTest.java| 179 +
 .../geode/internal/net/NioPlainEngineTest.java |  47 ++-
 .../geode/internal/net/NioSslEngineTest.java   | 432 +++--
 17 files changed, 1422 insertions(+), 486 deletions(-)

diff --git 
a/geode-core/src/distributedTest/java/org/apache/geode/internal/tcp/ConnectionCloseSSLTLSDUnitTest.java
 
b/geode-core/src/distributedTest/java/org/apache/geode/internal/tcp/ConnectionCloseSSLTLSDUnitTest.java
new file mode 100644
index 000..77fe9bf
--- /dev/null
+++ 
b/geode-core/src/distributedTest/java/org/apache/geode/internal/tcp/ConnectionCloseSSLTLSDUnitTest.java
@@ -0,0 +1,238 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional 
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY 
KIND, either express
+ * or implied. See the License for the specific language governing permissions 
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.internal.tcp;
+
+import static 
org.apache.geode.distributed.ConfigurationProperties.CONSERVE_SOCKETS;
+import static 
org.apache.geode.distributed.ConfigurationProperties.ENABLE_CLUSTER_CONFIGURATION;
+import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
+import static org.apache.geode.distributed.ConfigurationProperties.NAME;
+import static 
org.apache.geode.distributed.ConfigurationProperties.SOCKET_BUFFER_SIZE;
+import static 
org.apache.geode.distributed.ConfigurationProperties.SOCKET_LEASE_TIME;
+import static 
org.apache.geode.distributed.ConfigurationProperties.SSL_ENABLED_COMPONENTS;
+import static 
org.apache.geode.distributed.ConfigurationProperties.SSL_KEYSTORE;
+import static 
org.apache.geode.distributed.ConfigurationProperties.SSL_KEYSTORE_PASSWORD;

[geode] 03/03: * GEODE-8652: NioSslEngine.close() Bypasses Locks (#5712)

2020-11-06 Thread burcham
This is an automated email from the ASF dual-hosted git repository.

burcham pushed a commit to branch support/1.13
in repository https://gitbox.apache.org/repos/asf/geode.git

commit 32a8a55df4c0a5b8b1e5cfb746070804cbce5e0a
Author: Bill Burcham 
AuthorDate: Thu Nov 5 17:30:29 2020 -0800

* GEODE-8652: NioSslEngine.close() Bypasses Locks (#5712)

- NioSslEngine.close() proceeds even if readers (or writers) are
  operating on its ByteBuffers, allowing Connection.close() to close
  its socket and proceed.

- NioSslEngine.close() needed a lock only on the output buffer, so
  we split what was a single lock into two. Also instead of using
  synchronized we use a ReentrantLock so we can
  call tryLock() and time out if needed in NioSslEngine.close().

- Since readers/writers may hold locks on these input/output buffers
  when NioSslEngine.close() is called a reference count is maintained
  and the buffers are returned to the pool only when the last user
  is done.

- To manage the locking and reference counting a new AutoCloseable
  ByteBufferSharing interface is introduced with a trivial
  implementation: ByteBufferSharingNoOp and a real implementation:
  ByteBufferSharingImpl.

- Added a new unit test, and a new concurrency test for
  ByteBufferSharingImpl: both ensure that ByteBuffers are returned
  to the pool exactly once. Added a new DUnit test for the interaction
  between ByteBufferSharingImpl and NioSslEngine and Connection.

Co-authored-by: Bill Burcham 
Co-authored-by: Darrel Schneider 
Co-authored-by: Ernie Burghardt 
Co-authored-by: Dan Smith 
(cherry picked from commit af267c005a63317cbb8528cdb38eccf6a8747818)
---
 .../tcp/ConnectionCloseSSLTLSDUnitTest.java| 238 
 .../org/apache/geode/internal/tcp/server.keystore  | Bin 0 -> 1256 bytes
 ...LSocketHostNameVerificationIntegrationTest.java |   4 +-
 .../internal/net/SSLSocketIntegrationTest.java |  57 ++-
 .../apache/geode/codeAnalysis/excludedClasses.txt  |   1 +
 .../geode/internal/net/ByteBufferSharing.java  |  55 +++
 .../geode/internal/net/ByteBufferSharingImpl.java  | 168 
 .../geode/internal/net/ByteBufferSharingNoOp.java  |  52 +++
 .../org/apache/geode/internal/net/NioFilter.java   |  69 ++--
 .../apache/geode/internal/net/NioPlainEngine.java  |  27 +-
 .../apache/geode/internal/net/NioSslEngine.java| 365 -
 .../org/apache/geode/internal/tcp/Connection.java  |  34 +-
 .../org/apache/geode/internal/tcp/MsgReader.java   |  15 +-
 .../internal/net/ByteBufferConcurrencyTest.java| 165 
 .../internal/net/ByteBufferSharingImplTest.java| 179 +
 .../geode/internal/net/NioPlainEngineTest.java |  47 ++-
 .../geode/internal/net/NioSslEngineTest.java   | 432 +++--
 17 files changed, 1422 insertions(+), 486 deletions(-)

diff --git 
a/geode-core/src/distributedTest/java/org/apache/geode/internal/tcp/ConnectionCloseSSLTLSDUnitTest.java
 
b/geode-core/src/distributedTest/java/org/apache/geode/internal/tcp/ConnectionCloseSSLTLSDUnitTest.java
new file mode 100644
index 000..77fe9bf
--- /dev/null
+++ 
b/geode-core/src/distributedTest/java/org/apache/geode/internal/tcp/ConnectionCloseSSLTLSDUnitTest.java
@@ -0,0 +1,238 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional 
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY 
KIND, either express
+ * or implied. See the License for the specific language governing permissions 
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.internal.tcp;
+
+import static 
org.apache.geode.distributed.ConfigurationProperties.CONSERVE_SOCKETS;
+import static 
org.apache.geode.distributed.ConfigurationProperties.ENABLE_CLUSTER_CONFIGURATION;
+import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
+import static org.apache.geode.distributed.ConfigurationProperties.NAME;
+import static 
org.apache.geode.distributed.ConfigurationProperties.SOCKET_BUFFER_SIZE;
+import static 
org.apache.geode.distributed.ConfigurationProperties.SOCKET_LEASE_TIME;
+import static 
org.apache.geode.distributed.ConfigurationProperties.SSL_ENABLED_COMPONENTS;
+import static 
org.apache.geode.distributed.ConfigurationProperties.SSL_KEYSTORE;
+import static 
org.apache.geode.distributed.ConfigurationProperties.SSL_KEYSTORE_PASSWORD;