1.0.11: Fixed deadlock when child process reads from stdin

adamkewley · adamkewley · commit ced8e1c3398e · 2020-12-30T13:05:24.000Z
- Fixed a deadlock that can occur when the child process tries to read from the standard input - The deadlock occurs because jobson provides no standard input to child processes, but keeps the standard input open. This causes child processes to deadlock indefinitely when 'read'ing from stdin - This fix makes jobson 'close' the standard input of any child process at launch time. If any child process attempts to read from the standard input they'll read 0 bytes then stop - See #67 for more info - Thanks to @Russel88 for finding this bug!
diff --git a/.travis.yml b/.travis.yml
@@ -14,12 +14,13 @@ addons:
     - python3
     - python3-pip
     - python3-setuptools
+    - python3-venv
     - nodejs
     - docker
 before_install:
 - gem install fpm --version 1.10.0
-- pip3 install --user --upgrade pip virtualenv
-- virtualenv -p python3 venv
+- pip3 install --user --upgrade pip
+- python3 -m venv venv/
 - source venv/bin/activate
 - pip3 install -r jobson-docs/requirements.txt
 script: mvn package
diff --git a/README.md b/README.md
@@ -48,9 +48,9 @@ Requires java (8+):
 
 ```bash
 # install and add to PATH
-wget https://github.com/adamkewley/jobson/releases/download/1.0.10/jobson-nix-1.0.10.tar.gz
-tar xvf jobson-nix-1.0.10.tar.gz
-export PATH=$PATH:jobson-nix-1.0.10/bin
+wget https://github.com/adamkewley/jobson/releases/download/1.0.11/jobson-nix-1.0.11.tar.gz
+tar xvf jobson-nix-1.0.11.tar.gz
+export PATH=$PATH:jobson-nix-1.0.11/bin
 
 # create demo workspace
 jobson new --demo
diff --git a/jobson-deb/pom.xml b/jobson-deb/pom.xml
@@ -7,7 +7,7 @@
     <parent>
         <groupId>com.github.jobson</groupId>
         <artifactId>jobson-project</artifactId>
-        <version>1.0.10</version>
+        <version>1.0.11</version>
     </parent>
 
     <artifactId>jobson-deb</artifactId>
@@ -17,7 +17,7 @@
         <dependency>
             <groupId>com.github.jobson</groupId>
             <artifactId>jobson-nix</artifactId>
-            <version>1.0.10</version>
+            <version>1.0.11</version>
             <type>tar.gz</type>
         </dependency>
     </dependencies>
diff --git a/jobson-docker/pom.xml b/jobson-docker/pom.xml
@@ -7,7 +7,7 @@
     <parent>
         <groupId>com.github.jobson</groupId>
         <artifactId>jobson-project</artifactId>
-        <version>1.0.10</version>
+        <version>1.0.11</version>
     </parent>
 
     <artifactId>jobson-docker</artifactId>
@@ -17,7 +17,7 @@
         <dependency>
             <groupId>com.github.jobson</groupId>
             <artifactId>jobson-deb</artifactId>
-            <version>1.0.10</version>
+            <version>1.0.11</version>
             <type>deb</type>
         </dependency>
     </dependencies>
diff --git a/jobson-docs/pom.xml b/jobson-docs/pom.xml
@@ -7,11 +7,11 @@
     <parent>
         <groupId>com.github.jobson</groupId>
         <artifactId>jobson-project</artifactId>
-        <version>1.0.10</version>
+        <version>1.0.11</version>
     </parent>
 
     <artifactId>jobson-docs</artifactId>
-    <version>1.0.10</version>
+    <version>1.0.11</version>
     <packaging>pom</packaging>
 
     <dependencies>
diff --git a/jobson-nix/pom.xml b/jobson-nix/pom.xml
@@ -7,7 +7,7 @@
     <parent>
         <groupId>com.github.jobson</groupId>
         <artifactId>jobson-project</artifactId>
-        <version>1.0.10</version>
+        <version>1.0.11</version>
     </parent>
 
     <artifactId>jobson-nix</artifactId>
@@ -17,18 +17,18 @@
         <dependency>
             <groupId>com.github.jobson</groupId>
             <artifactId>jobson</artifactId>
-            <version>1.0.10</version>
+            <version>1.0.11</version>
         </dependency>
         <dependency>
             <groupId>com.github.jobson</groupId>
             <artifactId>jobson-docs</artifactId>
-            <version>1.0.10</version>
+            <version>1.0.11</version>
             <type>tar.gz</type>
         </dependency>
         <dependency>
             <groupId>com.github.jobson</groupId>
             <artifactId>jobson-ui</artifactId>
-            <version>1.0.10</version>
+            <version>1.0.11</version>
             <type>tar.gz</type>
         </dependency>
     </dependencies>
diff --git a/jobson-swagger-ui/pom.xml b/jobson-swagger-ui/pom.xml
@@ -7,11 +7,11 @@
     <parent>
         <groupId>com.github.jobson</groupId>
         <artifactId>jobson-project</artifactId>
-        <version>1.0.10</version>
+        <version>1.0.11</version>
     </parent>
 
     <artifactId>jobson-swagger-ui</artifactId>
-    <version>1.0.10</version>
+    <version>1.0.11</version>
     <packaging>pom</packaging>
 
     <dependencies>
diff --git a/jobson-swagger/pom.xml b/jobson-swagger/pom.xml
@@ -7,18 +7,18 @@
     <parent>
         <groupId>com.github.jobson</groupId>
         <artifactId>jobson-project</artifactId>
-        <version>1.0.10</version>
+        <version>1.0.11</version>
     </parent>
 
     <artifactId>jobson-swagger</artifactId>
-    <version>1.0.10</version>
+    <version>1.0.11</version>
     <packaging>pom</packaging>
 
     <dependencies>
         <dependency>
             <groupId>com.github.jobson</groupId>
             <artifactId>jobson</artifactId>
-            <version>1.0.10</version>
+            <version>1.0.11</version>
             <exclusions>
                 <exclusion>
                     <groupId>com.fasterxml</groupId>
diff --git a/jobson-ui/pom.xml b/jobson-ui/pom.xml
@@ -7,11 +7,11 @@
     <parent>
         <groupId>com.github.jobson</groupId>
         <artifactId>jobson-project</artifactId>
-        <version>1.0.10</version>
+        <version>1.0.11</version>
     </parent>
 
     <artifactId>jobson-ui</artifactId>
-    <version>1.0.10</version>
+    <version>1.0.11</version>
     <packaging>pom</packaging>
 
     <build>
diff --git a/jobson/pom.xml b/jobson/pom.xml
@@ -7,11 +7,11 @@
     <parent>
         <groupId>com.github.jobson</groupId>
         <artifactId>jobson-project</artifactId>
-        <version>1.0.10</version>
+        <version>1.0.11</version>
     </parent>
 
     <artifactId>jobson</artifactId>
-    <version>1.0.10</version>
+    <version>1.0.11</version>
     <packaging>jar</packaging>
 
     <dependencies>
diff --git a/jobson/src/main/java/com/github/jobson/jobs/LocalJobExecutor.java b/jobson/src/main/java/com/github/jobson/jobs/LocalJobExecutor.java
@@ -160,6 +160,15 @@ public CancelablePromise<JobExecutionResult> execute(PersistedJob req, JobEventL
 
             final Process runningProcess = processBuilder.start();
 
+            // close process's stdin stream. If this isn't done, the
+            // child process will block if it tries to read from stdin
+            // (because it's connected to jobson's stdin, which isn't
+            // being used)
+            //
+            // see https://github.com/adamkewley/jobson/issues/67 for
+            // a breakdown of the kinds of problems this can create
+            runningProcess.getOutputStream().close();
+
             log.info(req.getId() + ": launched: " + String.join(" ", argList));
 
             final SimpleCancelablePromise<JobExecutionResult> ret = new SimpleCancelablePromise<>();
diff --git a/jobson/src/test/java/com/github/jobson/jobs/execution/JobExecutorTest.java b/jobson/src/test/java/com/github/jobson/jobs/execution/JobExecutorTest.java
@@ -184,8 +184,7 @@ public void testExecutePromiseResolvesWithFatalErrorForFailingCommand() throws T
     @Test
     public void testExecutePromiseResolvesWithAbortedIfPromiseIsCancelled() throws Throwable {
         final JobExecutor jobExecutor = getInstance();
-        final PersistedJob req =
-                standardRequestWithCommand("cat"); // Long-running, because it blocks on an STDIN read.
+        final PersistedJob req = standardRequestWithCommand("sleep", "100");
         final CancelablePromise<JobExecutionResult> ret =
                 jobExecutor.execute(req, createNullListeners());
 
@@ -195,6 +194,28 @@ public void testExecutePromiseResolvesWithAbortedIfPromiseIsCancelled() throws T
                 () -> ret.cancel(true));
     }
 
+    @Test
+    public void testExecutePromiseResolvesCompletedIfApplicationReadsFromStdin() throws Throwable {
+        // jobson should close `stdin`, so that any application
+        // attempting to read from stdin reads nothing (rather than
+        // deadlocking indefinitely on content Jobson will never
+        // write)
+        //
+        // see: https://github.com/adamkewley/jobson/issues/67
+        //
+        // (abstract): user ran an application that reads from stdin
+        // in certain circumstances and Jobson was deadlocking on that
+        final JobExecutor jobExecutor = getInstance();
+        final PersistedJob req = standardRequestWithCommand("cat");  // reads from stdin
+        final CancelablePromise<JobExecutionResult> ret =
+                jobExecutor.execute(req, createNullListeners());
+
+        promiseAssert(
+                ret,
+                result -> assertThat(result.getFinalStatus()).isEqualTo(JobStatus.FINISHED),
+                () -> ret.cancel(true));
+    }
+
     @Test
     public void testExecutePromiseResolvesWithTheOutputsWrittenByTheApplication() throws Throwable {
         final JobExecutor jobExecutor = getInstance();
diff --git a/jobson/src/test/resources/fixtures/systemtests/jobspecs.yml b/jobson/src/test/resources/fixtures/systemtests/jobspecs.yml
@@ -71,8 +71,8 @@
   expectedInputs: []
 
   execution:
-    application: cat
-    arguments: []
+    application: sleep
+    arguments: ['100']
 
 - id: fourth-spec
   name: Spec that creates an output
diff --git a/pom.xml b/pom.xml
@@ -6,7 +6,7 @@
 
     <groupId>com.github.jobson</groupId>
     <artifactId>jobson-project</artifactId>
-    <version>1.0.10</version>
+    <version>1.0.11</version>
     <packaging>pom</packaging>
 
     <name>jobson project</name>
@@ -48,8 +48,8 @@
     </developers>
 
     <properties>
-        <version.jobsonswagger>1.0.10</version.jobsonswagger>
-        <version.jobsonswaggerui>1.0.10</version.jobsonswaggerui>
+        <version.jobsonswagger>1.0.11</version.jobsonswagger>
+        <version.jobsonswaggerui>1.0.11</version.jobsonswaggerui>
 
         <dropwizard.version>2.0.10</dropwizard.version>
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
