Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jpos3 ISOServer shutdown thread deadlock #631

Closed
swyow opened this issue Dec 31, 2024 · 4 comments
Closed

jpos3 ISOServer shutdown thread deadlock #631

swyow opened this issue Dec 31, 2024 · 4 comments

Comments

@swyow
Copy link

swyow commented Dec 31, 2024

org.jpos.iso.ISOServer

public void shutdown () {
    shutdown = true;
    executor.submit(() -> {
        Thread.currentThread().setName("ISOServer-shutdown");
        shutdownServer();
        if (!cfg.getBoolean("keep-channels")) {
            shutdownChannels();
        }
    });
}
private void shutdownServer () {
    try {
        if (serverSocket != null) {
            serverSocket.close ();
            fireEvent(new ISOServerShutdownEvent(this));
        }
        executor.awaitTermination(LONG_RELAX, TimeUnit.SECONDS);
    } catch (IOException | InterruptedException e) {
        fireEvent(new ISOServerShutdownEvent(this));
        Logger.log (new LogEvent (this, "shutdown", e));
    }
}

Executor.awaitTermination(LONG_RELAX, TimeUnit.SECONDS) in shutdownServer() will not succeed until a timeout occurs.

It is trying to wait for its own thread to finish. The executor submits a task, and the task waits for the executor to complete all tasks, resulting in a deadlock. Only after the LONG_RELAX timeout may the code continue to shut down channels.

And LONG_RELAX is 5000 seconds, which is too long before the channels can be shut down.
The suggestion fix is to remove awaitTermination, or to break the shutdown server and shutdown channel into two threads, with only the first completing before the second is called.

Please advise.

@ar
Copy link
Member

ar commented Jan 3, 2025

Need to test it but at first sight you have a very valid point. The awaitTermination needs to go up inside the shutdown method.

@swyow
Copy link
Author

swyow commented Jan 3, 2025

It has been tried in our lab, and because we need to dynamically start/stop QServer, it prevented us from upgrading to jpos3, hopefully to get a patch soon, thanks.

@ar ar closed this as completed in a17bafd Jan 5, 2025
@ar
Copy link
Member

ar commented Jan 5, 2025

Issue has been confirmed and fixed in a17bafd. Feel free to reopen @swyow if the fix doesn't work for you.

@swyow
Copy link
Author

swyow commented Jan 6, 2025

tested and working, thanks @ar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants