[
https://issues.apache.org/jira/browse/TIKA-4704?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18071525#comment-18071525
]
ASF GitHub Bot commented on TIKA-4704:
--------------------------------------
Copilot commented on code in PR #2746:
URL: https://github.com/apache/tika/pull/2746#discussion_r3041397694
##########
tika-grpc/src/main/java/org/apache/tika/pipes/grpc/TikaGrpcServerImpl.java:
##########
@@ -488,5 +488,14 @@ public void shutdown() {
LOG.error("Error shutting down Ignite server", e);
}
}
+ if (pipesClient != null) {
+ LOG.info("Shutting down the pipes client");
+ try {
+ pipesClient.close();
+ }
+ catch (IOException e) {
Review Comment:
The `catch` is formatted as `}
catch (...)` which is inconsistent with the rest of this file (`} catch
(...)`) and will likely be reformatted/flagged by Spotless. Keep `catch` on the
same line as the closing brace.
```suggestion
} catch (IOException e) {
```
##########
tika-grpc/src/main/java/org/apache/tika/pipes/grpc/TikaGrpcServerImpl.java:
##########
@@ -488,5 +488,14 @@ public void shutdown() {
LOG.error("Error shutting down Ignite server", e);
}
}
+ if (pipesClient != null) {
+ LOG.info("Shutting down the pipes client");
+ try {
+ pipesClient.close();
+ }
+ catch (IOException e) {
+ LOG.error("Error closing the pipes client", e);
+ }
+ }
Review Comment:
`TikaGrpcServer.stop()` currently calls `serviceImpl.shutdown()` before
`server.shutdown().awaitTermination(...)`. With this change, closing
`pipesClient` here can interrupt in-flight RPCs during a graceful shutdown
window. Consider deferring `pipesClient.close()` until after the gRPC `Server`
has terminated (or adjusting the shutdown sequencing) to avoid spurious request
failures during shutdown.
##########
tika-grpc/src/main/java/org/apache/tika/pipes/grpc/TikaGrpcServerImpl.java:
##########
@@ -488,5 +488,14 @@ public void shutdown() {
LOG.error("Error shutting down Ignite server", e);
}
}
+ if (pipesClient != null) {
+ LOG.info("Shutting down the pipes client");
+ try {
+ pipesClient.close();
+ }
+ catch (IOException e) {
+ LOG.error("Error closing the pipes client", e);
Review Comment:
After successfully closing `pipesClient`, the field should be set to null
(ideally in a `finally`) to make shutdown idempotent and avoid repeated close
attempts or use-after-close from other code paths. This also matches the
pattern used above for `igniteStoreServer`.
```suggestion
} catch (IOException e) {
LOG.error("Error closing the pipes client", e);
} finally {
pipesClient = null;
```
> Avoid remaining temp files
> --------------------------
>
> Key: TIKA-4704
> URL: https://issues.apache.org/jira/browse/TIKA-4704
> Project: Tika
> Issue Type: Task
> Affects Versions: 3.3.0
> Reporter: Tilman Hausherr
> Priority: Minor
> Fix For: 4.0.0, 3.3.1
>
> Attachments: screenshot-1.png
>
>
> This is my temp directory after a successful build of tika 3. We should try
> to lessen this.
> !screenshot-1.png!
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
