[tika] branch branch_1x updated: [TIKA-3825] ForkClient to check for thread interrupted status when waiting for response. Add test to ForkParserTest to demonstrate issue and fix (#633)

[ta...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

tallison pushed a commit to branch branch_1x
in repository https://gitbox.apache.org/repos/asf/tika.git


The following commit(s) were added to refs/heads/branch_1x by this push:
     new ae61117c9 [TIKA-3825] ForkClient to check for thread interrupted status when waiting for response. Add test to ForkParserTest to demonstrate issue and fix (#633)
ae61117c9 is described below

commit ae61117c9436d27e23c97f2270f59ea98a624644
Author: Ben Gilbert <Th...@users.noreply.github.com>
AuthorDate: Mon Aug 1 22:17:34 2022 +0100

    [TIKA-3825] ForkClient to check for thread interrupted status when waiting for response. Add test to ForkParserTest to demonstrate issue and fix (#633)
---
 .../main/java/org/apache/tika/fork/ForkClient.java |  3 +-
 .../java/org/apache/tika/fork/ForkParserTest.java  | 39 ++++++++++++++++++++++
 .../java/org/apache/tika/fork/ForkTestParser.java  | 15 ++++++++-
 3 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/tika-core/src/main/java/org/apache/tika/fork/ForkClient.java b/tika-core/src/main/java/org/apache/tika/fork/ForkClient.java
index 82396196b..1b7b6f007 100644
--- a/tika-core/src/main/java/org/apache/tika/fork/ForkClient.java
+++ b/tika-core/src/main/java/org/apache/tika/fork/ForkClient.java
@@ -279,7 +279,7 @@ class ForkClient {
     private Throwable waitForResponse(List<ForkResource> resources)
             throws IOException {
         output.flush();
-        while (true) {
+        while (!Thread.currentThread().isInterrupted()) {
             int type = input.read();
             if (type == -1) {
                 throw new IOException(
@@ -300,6 +300,7 @@ class ForkClient {
                 return null;
             }
         }
+        throw new IOException(new InterruptedException());
     }
 
     /**
diff --git a/tika-core/src/test/java/org/apache/tika/fork/ForkParserTest.java b/tika-core/src/test/java/org/apache/tika/fork/ForkParserTest.java
index d450825fd..06f44da80 100644
--- a/tika-core/src/test/java/org/apache/tika/fork/ForkParserTest.java
+++ b/tika-core/src/test/java/org/apache/tika/fork/ForkParserTest.java
@@ -37,9 +37,15 @@ import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
 import java.util.concurrent.Semaphore;
+import java.util.concurrent.TimeUnit;
 
 import org.apache.tika.TikaTest;
 import org.apache.tika.exception.TikaException;
@@ -460,6 +466,39 @@ public class ForkParserTest extends TikaTest {
         proxy.skippedEntity(sb.toString());
     }
 
+    @Test
+    public void testForkParserDoesntPreventShutdown() throws Exception {
+        ExecutorService service = Executors.newFixedThreadPool(1);
+        CountDownLatch cdl = new CountDownLatch(1);
+        service.submit(() -> {
+            try (ForkParser parser = new ForkParser(ForkParserTest.class.getClassLoader(),
+                    new ForkTestParser.ForkTestParserWaiting())) {
+                Metadata metadata = new Metadata();
+                ContentHandler output = new BodyContentHandler();
+                InputStream stream = new ByteArrayInputStream(new byte[0]);
+                ParseContext context = new ParseContext();
+                cdl.countDown();
+                parser.parse(stream, output, metadata, context);
+                // Don't care about output not planning to get this far
+            } catch (IOException | SAXException | TikaException e) {
+                throw new RuntimeException(e);
+            }
+        });
+        // Wait to make sure submitted runnable is actually running
+        boolean await = cdl.await(1, TimeUnit.SECONDS);
+        if (!await) {
+            // This should never happen but be thorough
+            fail("Future never ran so cannot test cancellation");
+        }
+        // Parse is being called try and shutdown
+        Instant requestShutdown = Instant.now();
+        service.shutdownNow();
+        service.awaitTermination(15, TimeUnit.SECONDS);
+        long secondsSinceShutdown = ChronoUnit.SECONDS.between(requestShutdown, Instant.now());
+        assertTrue(secondsSinceShutdown < 5, "Should have shutdown the service in less than 5 seconds");
+    }
+
+
     //use this to test that the wrapper handler is acted upon by the server but not proxied back
     private static class ToFileHandler extends AbstractRecursiveParserWrapperHandler {
 
diff --git a/tika-core/src/test/java/org/apache/tika/fork/ForkTestParser.java b/tika-core/src/test/java/org/apache/tika/fork/ForkTestParser.java
index 3f3c9c2bd..9e7612bd7 100644
--- a/tika-core/src/test/java/org/apache/tika/fork/ForkTestParser.java
+++ b/tika-core/src/test/java/org/apache/tika/fork/ForkTestParser.java
@@ -64,4 +64,17 @@ class ForkTestParser extends AbstractParser {
             super.parse(stream, handler, metadata, context);
         }
     }
-}
\ No newline at end of file
+
+    static class ForkTestParserWaiting extends ForkTestParser {
+        @Override
+        public void parse(InputStream stream, ContentHandler handler, Metadata metadata,
+                          ParseContext context) throws IOException, SAXException, TikaException {
+            try {
+                Thread.sleep(10_000);
+            } catch (InterruptedException e) {
+                throw new RuntimeException(e);
+            }
+            super.parse(stream, handler, metadata, context);
+        }
+    }
+}