Issue 4383 - changes from review

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java

@@ -56,19 +56,25 @@
  * <p>
  * Deleting the parts can be done from a different thread if the parts are parsed asynchronously.
  * Because of this we use the state to fail the parsing and coordinate which thread will delete any remaining parts.
- * The deletion of parts is done by the cleanup thread in all cases except the transition from ERROR-&gt;DELETED which
+ * The deletion of parts is done by the cleanup thread in all cases except the transition from DELETING-&gt;DELETED which
  * is done by the parsing thread.
  * </p>
  * <pre>{@code
+ * UNPARSED - Parsing has not started, there are no parts which need to be cleaned up.
+ * PARSING  - The parsing thread is reading from the InputStream and generating parts.
+ * PARSED   - Parsing has complete and no more parts will be generated.
+ * DELETING - deleteParts() has been called while we were in PARSING state, parsing thread will do the delete.
+ * DELETED  - The parts have been deleted, this is the terminal state.
+ *
  *                              deleteParts()
  *     +--------------------------------------------------------------+
  *     |                                                              |
  *     |                                          deleteParts()       v
  *  UNPARSED -------> PARSING --------> PARSED  ------------------>DELETED
  *                      |                                             ^
  *                      |                                             |
- *                      +----------------> ERROR ---------------------+
- *                        deleteParts()             parsing thread
+ *                      +---------------> DELETING -------------------+
+ *                        deleteParts()               parsing thread
  * }</pre>
  * @see <a href="https://tools.ietf.org/html/rfc7578">https://tools.ietf.org/html/rfc7578</a>
  */
@@ -79,8 +85,8 @@ private enum State
         UNPARSED,
         PARSING,
         PARSED,
-        DELETED,
-        ERROR
+        DELETING,
+        DELETED
     }
 
     private static final Logger LOG = Log.getLogger(MultiPartFormInputStream.class);
@@ -406,11 +412,11 @@ public void deleteParts()
             switch (state)
             {
                 case DELETED:
-                case ERROR:
+                case DELETING:
                     return;
 
                 case PARSING:
-                    state = State.ERROR;
+                    state = State.DELETING;
                     return;
 
                 case UNPARSED:
@@ -423,10 +429,10 @@ public void deleteParts()
             }
         }
 
-        uncheckedDeleteParts();
+        delete();
     }
 
-    private void uncheckedDeleteParts()
+    private void delete()
     {
         MultiException err = null;
         for (List<Part> parts : _parts.values())
@@ -631,7 +637,7 @@ else if (len == -1)
                         state = State.PARSED;
                         break;
 
-                    case ERROR:
+                    case DELETING:
                         state = State.DELETED;
                         cleanup = true;
                         break;
@@ -642,7 +648,7 @@ else if (len == -1)
             }
 
             if (cleanup)
-                uncheckedDeleteParts();
+                delete();
         }
     }
 

jetty-server/src/test/java/org/eclipse/jetty/server/MultiPartFormInputStreamTest.java

@@ -563,7 +563,7 @@ public int read() throws IOException
 
         // The call to getParts should throw an error.
         Throwable error = assertThrows(IOException.class, mpis::getParts);
-        assertThat(error.getMessage(), is("ERROR"));
+        assertThat(error.getMessage(), is("DELETING"));
 
         // There was no error with the cleanup.
         assertNull(cleanupError.get());