You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2021/02/09 00:05:26 UTC
[GitHub] [geode] echobravopapa commented on a change in pull request #5987: GEODE-8817: server hangs in cache.close()
echobravopapa commented on a change in pull request #5987:
URL: https://github.com/apache/geode/pull/5987#discussion_r572463893
##########
File path: geode-core/src/main/java/org/apache/geode/internal/net/SocketCloser.java
##########
@@ -163,60 +164,73 @@ private Future asyncExecute(String address, Runnable runnableToExecute) {
}
/**
- * Closes the specified socket in a background thread. In some cases we see close hang (see bug
- * 33665). Depending on how the SocketCloser is configured (see ASYNC_CLOSE_WAIT_MILLISECONDS)
+ * Closes the specified socket in a background thread. In some cases we see close hang.
+ * Depending on how the SocketCloser is configured (see ASYNC_CLOSE_WAIT_MILLISECONDS)
+ * this method may block for a certain amount of time. If it is called after the SocketCloser is
+ * closed then a normal synchronous close is done.
+ *
+ * @param socket the socket to close
+ * @param address key used to determine which executor to use. Usually the address of a peer or
+ * client
+ * @param runBeforeClose a Runnable with stuff to execute before the socket is closed
+ */
+ public void asyncClose(final Socket socket, final String address, final Runnable runBeforeClose) {
+ asyncClose(socket, address, runBeforeClose, () -> {
+ });
+ }
+
+ /**
+ * Closes the specified socket in a background thread. In some cases we see close hang.
+ * Depending on how the SocketCloser is configured (see ASYNC_CLOSE_WAIT_MILLISECONDS)
* this method may block for a certain amount of time. If it is called after the SocketCloser is
* closed then a normal synchronous close is done.
*
* @param socket the socket to close
- * @param address identifies who the socket is connected to
- * @param extra an optional Runnable with stuff to execute before the socket is closed
+ * @param address key used to determine which executor to use. Usually the address of a peer or
+ * client
+ * @param runBeforeClose a Runnable with stuff to execute before the socket is closed
+ * @param runAfterClose a Runnable with stuff to execute after the socket is closed
*/
- public void asyncClose(final Socket socket, final String address, final Runnable extra) {
- if (socket == null || socket.isClosed()) {
+ public void asyncClose(@NotNull final Socket socket, @NotNull final String address,
Review comment:
NotNull 🚀
##########
File path: geode-core/src/main/java/org/apache/geode/internal/net/SocketCloser.java
##########
@@ -163,60 +164,73 @@ private Future asyncExecute(String address, Runnable runnableToExecute) {
}
/**
- * Closes the specified socket in a background thread. In some cases we see close hang (see bug
- * 33665). Depending on how the SocketCloser is configured (see ASYNC_CLOSE_WAIT_MILLISECONDS)
+ * Closes the specified socket in a background thread. In some cases we see close hang.
+ * Depending on how the SocketCloser is configured (see ASYNC_CLOSE_WAIT_MILLISECONDS)
+ * this method may block for a certain amount of time. If it is called after the SocketCloser is
+ * closed then a normal synchronous close is done.
+ *
+ * @param socket the socket to close
+ * @param address key used to determine which executor to use. Usually the address of a peer or
+ * client
+ * @param runBeforeClose a Runnable with stuff to execute before the socket is closed
+ */
+ public void asyncClose(final Socket socket, final String address, final Runnable runBeforeClose) {
+ asyncClose(socket, address, runBeforeClose, () -> {
+ });
+ }
+
+ /**
+ * Closes the specified socket in a background thread. In some cases we see close hang.
+ * Depending on how the SocketCloser is configured (see ASYNC_CLOSE_WAIT_MILLISECONDS)
* this method may block for a certain amount of time. If it is called after the SocketCloser is
* closed then a normal synchronous close is done.
*
* @param socket the socket to close
- * @param address identifies who the socket is connected to
- * @param extra an optional Runnable with stuff to execute before the socket is closed
+ * @param address key used to determine which executor to use. Usually the address of a peer or
+ * client
+ * @param runBeforeClose a Runnable with stuff to execute before the socket is closed
+ * @param runAfterClose a Runnable with stuff to execute after the socket is closed
*/
- public void asyncClose(final Socket socket, final String address, final Runnable extra) {
- if (socket == null || socket.isClosed()) {
+ public void asyncClose(@NotNull final Socket socket, @NotNull final String address,
+ @NotNull final Runnable runBeforeClose,
+ @NotNull final Runnable runAfterClose) {
+ if (socket.isClosed()) {
return;
}
boolean doItInline = false;
try {
- Future submittedTask = null;
+ Future submittedTask = CompletableFuture.completedFuture(this);
closedLock.lock();
try {
if (closed) {
// this SocketCloser has been closed so do a synchronous, inline, close
doItInline = true;
} else {
- submittedTask = asyncExecute(address, new Runnable() {
- @Override
- public void run() {
- Thread.currentThread().setName("AsyncSocketCloser for " + address);
- try {
- if (extra != null) {
- extra.run();
- }
- inlineClose(socket);
- } finally {
- Thread.currentThread().setName("unused AsyncSocketCloser");
- }
+ submittedTask = asyncExecute(address, () -> {
+ Thread.currentThread().setName("AsyncSocketCloser for " + address);
+ try {
+ runBeforeClose.run();
+ inlineClose(socket);
+ runAfterClose.run();
+ } finally {
+ Thread.currentThread().setName("unused AsyncSocketCloser");
}
});
}
} finally {
closedLock.unlock();
}
- if (submittedTask != null) {
+ if (!doItInline) {
waitForFutureTaskWithTimeout(submittedTask);
+ return;
}
} catch (RejectedExecutionException | OutOfMemoryError ignore) {
// If we can't start a thread to close the socket just do it inline.
Review comment:
maybe delete these comments now ...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org