Skip to content

Commit

Permalink
revise Disposer regarding exception handling
Browse files Browse the repository at this point in the history
  • Loading branch information
t-horikawa committed Nov 18, 2024
1 parent a428aa2 commit dc9ee63
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
* limitations under the License.
*/
package com.tsurugidb.tsubakuro.channel.common.connection;

import java.io.IOException;
import java.io.UncheckedIOException;
import java.util.ArrayDeque;
import java.util.Queue;
import java.util.concurrent.TimeoutException;
Expand All @@ -25,6 +28,7 @@

import com.tsurugidb.tsubakuro.channel.common.connection.wire.impl.ChannelResponse;
import com.tsurugidb.tsubakuro.client.SessionAlreadyClosedException;
import com.tsurugidb.tsubakuro.exception.ServerException;

Check warning on line 31 in modules/common/src/main/java/com/tsurugidb/tsubakuro/channel/common/connection/Disposer.java

View workflow job for this annotation

GitHub Actions / Checkstyle-pr

imports.UnusedImportsCheck

Unused import - com.tsurugidb.tsubakuro.exception.ServerException.

Check warning on line 31 in modules/common/src/main/java/com/tsurugidb/tsubakuro/channel/common/connection/Disposer.java

View workflow job for this annotation

GitHub Actions / Checkstyle

imports.UnusedImportsCheck

Unused import - com.tsurugidb.tsubakuro.exception.ServerException.
import com.tsurugidb.tsubakuro.util.ServerResource;

/**
Expand All @@ -47,8 +51,9 @@ public class Disposer extends Thread {
public interface CleanUp {
/**
* clean up procedure.
* @throws IOException An error was occurred while cleanUP() is executed.
*/
void cleanUp();
void cleanUp() throws IOException;
}

/**
Expand All @@ -59,6 +64,8 @@ public Disposer() {

@Override
public void run() {
Exception exception = null;

while (true) {
ForegroundFutureResponse<?> futureResponse;
synchronized (queue) {
Expand All @@ -75,16 +82,22 @@ public void run() {
// Server resource has not created at the server
continue;
} catch (SessionAlreadyClosedException e) {
// Server resource has been disposed by the session close
throw new AssertionError("SessionAlreadyClosedException should not occur in the current server implementation"); // FIXME remove this line
// continue;
if (exception == null) {
exception = e;
} else {
exception.addSuppressed(e);
}
continue;
} catch (TimeoutException e) {
// Let's try again
queue.add(futureResponse);
continue;
} catch (Exception e) {
// should not occur
LOG.info(e.getMessage());
} catch (Exception e) { // should not occur
if (exception == null) {
exception = e;
} else {
exception.addSuppressed(e);
}
continue;
}
} else {
Expand All @@ -98,8 +111,21 @@ public void run() {
// No problem, it's OK
}
}
cleanUp.get().cleanUp();
notifyFinish();

try {
cleanUp.get().cleanUp();
} catch (IOException e) {
if (exception == null) {
exception = e;
} else {
exception.addSuppressed(e);
}
}

if (exception != null) {
LOG.info(exception.getMessage());
throw new UncheckedIOException(new IOException(exception));
}
}

void add(ForegroundFutureResponse<?> futureResponse) {
Expand All @@ -116,8 +142,9 @@ void add(ForegroundFutureResponse<?> futureResponse) {
* If disposer thread has not started, cleanUp() is executed.
* NOTE: This method is assumed to be called only in close and/or shutdown of a Session.
* @param c the clean up procesure to be registered
* @throws IOException An error was occurred when CleanUP c has been immediately executed.
*/
public void registerCleanUp(CleanUp c) {
public void registerCleanUp(CleanUp c) throws IOException {
if (!started.getAndSet(true)) {
c.cleanUp();
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,20 +284,15 @@ public void close() throws IOException, ServerException, InterruptedException {
}
future.close();
}
public synchronized void cleanUp() {
try {
if (future == dummyFuture) {
var shutdownMessageBuilder = CoreRequest.Shutdown.newBuilder();
future = sendUrgent(
SERVICE_ID,
toDelimitedByteArray(newRequest()
.setShutdown(shutdownMessageBuilder.setType(type.type()))
.build()),
new ShutdownProcessor().asResponseProcessor());
}
} catch (Exception e) {
// FIXME how to handle exceptions occuered in background thread?
throw new AssertionError(e.getMessage());
public synchronized void cleanUp() throws IOException {
if (future == dummyFuture) {
var shutdownMessageBuilder = CoreRequest.Shutdown.newBuilder();
future = sendUrgent(
SERVICE_ID,
toDelimitedByteArray(newRequest()
.setShutdown(shutdownMessageBuilder.setType(type.type()))
.build()),
new ShutdownProcessor().asResponseProcessor());
}
}
}
Expand All @@ -324,12 +319,11 @@ public Timeout getCloseTimeout() {
}

class CloseCleanUp implements Disposer.CleanUp {
public void cleanUp() {
public void cleanUp() throws IOException {
try {
doClose();
} catch (ServerException | IOException | InterruptedException e) {
// FIXME how to handle exceptions occuered in background thread?
throw new AssertionError(e.getMessage());
} catch (ServerException | InterruptedException e) {
throw new IOException(e);
}
}
}
Expand Down

0 comments on commit dc9ee63

Please sign in to comment.