Skip to content

Update java/Ice/greeter shutdown #332

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Mar 4, 2025
Merged

Update java/Ice/greeter shutdown #332

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aa6aa41
Update java/Ice/greeter shutdown
bernardnormier Mar 3, 2025
3cc14ab
Fix review comment
bernardnormier Mar 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion java/Ice/greeter/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ First, start one of the server implementations:
### Windows

```shell
server\build\install\server\binserver
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to answer your question, this is the default location where the application plugin we use puts these executables. Maybe we could tweak it, but this is the default.

If you're a normal Java developer, used to gradle, what you would expect to do is this:

./gradlew :server:run
./gradlew :client:run

And this does work, but it also outputs extra clutter which obscures what the demo is actually doing.

You can remove this clutter by running:

./gradlew :server:run --quiet
./gradlew :client:run --quiet

Which I think is slightly clearer than having to type out the paths directly.
So, I think we should switch to recommending these commands instead.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we change this because how Ctrl-C is handled by the scripts.

server\build\install\server\bin\server
```

or
Expand Down
28 changes: 21 additions & 7 deletions java/Ice/greeter/server/src/main/java/com/zeroc/demos/greeter/Server.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,18 @@

package com.zeroc.demos.greeter;

import com.zeroc.demos.greeter.Chatbot;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't need import since Chatbot is in the same package.

import com.zeroc.Ice.Communicator;
import com.zeroc.Ice.ObjectAdapter;

class Server {
public static void main(String[] args) {
// Create an Ice communicator to initialize the Ice runtime. The communicator is disposed before the program exits.
// Create an Ice communicator to initialize the Ice runtime. The communicator is closed (destroyed) at the end
// of the try statement.
try (Communicator communicator = com.zeroc.Ice.Util.initialize(args)) {
Runtime.getRuntime().addShutdownHook(new Thread(() -> {
communicator.destroy();
}));

// Register a shutdown hook that calls communicator.shutdown() when the user shuts down the server with
// Ctrl+C or similar. The shutdown hook thread also waits until the current thread completes its cleanup.
shutdownCommunicatorOnCtrlC(communicator, Thread.currentThread());

// Create an object adapter that listens for incoming requests and dispatches them to servants.
ObjectAdapter adapter = communicator.createObjectAdapterWithEndpoints("GreeterAdapter", "tcp -p 4061");
Expand All @@ -24,9 +25,22 @@ public static void main(String[] args) {
adapter.activate();
System.out.println("Listening on port 4061...");

// Wait until the user presses Ctrl+C.
// Wait until the communicator is shut down. Here, this occurs when the user presses Ctrl+C.
communicator.waitForShutdown();
System.out.println("Caught Ctrl+C, exiting...");
}
}

private static void shutdownCommunicatorOnCtrlC(Communicator communicator, Thread cleanupThread) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like cleanupThread name very much, what about using just mainThread?

Copy link
Member

@externl externl Mar 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also prefer mainThread.

It's not obvious from the name cleanupThread that your intention is that a user would do cleanup in the main thread, since we don't do any. It also seems to go against the convention in Java that you would do the cleanup in this hook.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we don't do any.

We do perform cleanup in this thread: the cleanup is the destruction of the communicator that occurs at the end of the try-with-resources statement.

It also seems to go against the convention in Java that you would do the cleanup in this hook.

What do you suggest then?

This PR is all about NOT performing the cleanup in the hook, but waiting in the hook for some other thread (the cleanup thread) to complete the cleanup.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like Jose suggested, I would just call it mainThread.

Runtime.getRuntime().addShutdownHook(new Thread(() -> {
System.out.println("Caught Ctrl+C, shutting down...");
communicator.shutdown();

// Wait until the cleanup thread completes.
try {
cleanupThread.join();
} catch (InterruptedException e) {
assert false : "Shutdown thread cannot be interrupted";
}
}));
}
}
28 changes: 21 additions & 7 deletions java/Ice/greeter/serveramd/src/main/java/com/zeroc/demos/greeter/Server.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,18 @@

package com.zeroc.demos.greeter;

import com.zeroc.demos.greeter.Chatbot;
import com.zeroc.Ice.Communicator;
import com.zeroc.Ice.ObjectAdapter;

class Server {
public static void main(String[] args) {
// Create an Ice communicator to initialize the Ice runtime. The communicator is disposed before the program exits.
// Create an Ice communicator to initialize the Ice runtime. The communicator is closed (destroyed) at the end
// of the try statement.
try (Communicator communicator = com.zeroc.Ice.Util.initialize(args)) {
Runtime.getRuntime().addShutdownHook(new Thread(() -> {
communicator.destroy();
}));

// Register a shutdown hook that calls communicator.shutdown() when the user shuts down the server with
// Ctrl+C or similar. The shutdown hook thread also waits until the current thread completes its cleanup.
shutdownCommunicatorOnCtrlC(communicator, Thread.currentThread());

// Create an object adapter that listens for incoming requests and dispatches them to servants.
ObjectAdapter adapter = communicator.createObjectAdapterWithEndpoints("GreeterAdapter", "tcp -p 4061");
Expand All @@ -24,9 +25,22 @@ public static void main(String[] args) {
adapter.activate();
System.out.println("Listening on port 4061...");

// Wait until the user presses Ctrl+C.
// Wait until the communicator is shut down. Here, this occurs when the user presses Ctrl+C.
communicator.waitForShutdown();
System.out.println("Caught Ctrl+C, exiting...");
}
}

private static void shutdownCommunicatorOnCtrlC(Communicator communicator, Thread cleanupThread) {
Runtime.getRuntime().addShutdownHook(new Thread(() -> {
System.out.println("Caught Ctrl+C, shutting down...");
communicator.shutdown();

// Wait until the cleanup thread completes.
try {
cleanupThread.join();
} catch (InterruptedException e) {
assert false : "Shutdown thread cannot be interrupted";
}
}));
}
}
Loading