diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java b/jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java
index 8a53207e2f24..d04c7b62a316 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java
@@ -52,7 +52,24 @@
* MultiPartInputStream
*
* Handle a MultiPart Mime input stream, breaking it up on the boundary into files and strings.
- *
+ *
+ *
+ * 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->DELETED which
+ * is done by the parsing thread.
+ *
+ *
+ * deleteParts()
+ * +--------------------------------------------------------------+
+ * | |
+ * | deleteParts() v
+ * UNPARSED -------> PARSING --------> PARSED ------------------>DELETED
+ * | ^
+ * | |
+ * +----------------> ERROR ---------------------+
+ * deleteParts() parsing thread
+ *
* @see https://tools.ietf.org/html/rfc7578
*/
public class MultiPartFormInputStream
@@ -62,8 +79,8 @@ private enum State
UNPARSED,
PARSING,
PARSED,
- CLOSING,
- CLOSED
+ DELETED,
+ ERROR
}
private static final Logger LOG = Log.getLogger(MultiPartFormInputStream.class);
@@ -384,22 +401,29 @@ public void deleteParts()
{
switch (state)
{
- case CLOSED:
- case UNPARSED:
- state = State.CLOSED;
+ case DELETED:
+ case ERROR:
return;
case PARSING:
- state = State.CLOSING;
+ state = State.ERROR;
+ return;
+
+ case UNPARSED:
+ state = State.DELETED;
return;
case PARSED:
- case CLOSING:
- state = State.CLOSED;
+ state = State.DELETED;
break;
}
}
+ uncheckedDeleteParts();
+ }
+
+ private void uncheckedDeleteParts()
+ {
MultiException err = null;
for (List parts : _parts.values())
{
@@ -488,7 +512,7 @@ protected void parse()
return;
default:
- _err = new IllegalStateException(state.name());
+ _err = new IOException(state.name());
return;
}
}
@@ -537,7 +561,7 @@ else if ("".equals(_config.getLocation()))
{
if (state != State.PARSING)
{
- _err = new IllegalStateException(state.name());
+ _err = new IOException(state.name());
return;
}
}
@@ -607,7 +631,8 @@ else if (len == -1)
state = State.PARSED;
break;
- case CLOSING:
+ case ERROR:
+ state = State.DELETED;
cleanup = true;
break;
@@ -617,7 +642,7 @@ else if (len == -1)
}
if (cleanup)
- deleteParts();
+ uncheckedDeleteParts();
}
}
diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/MultiPartFormInputStreamTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/MultiPartFormInputStreamTest.java
index 4862e32b62e4..4854445e6be0 100644
--- a/jetty-server/src/test/java/org/eclipse/jetty/server/MultiPartFormInputStreamTest.java
+++ b/jetty-server/src/test/java/org/eclipse/jetty/server/MultiPartFormInputStreamTest.java
@@ -564,8 +564,8 @@ public int read() throws IOException
}).start();
// The call to getParts should throw an error.
- Throwable error = assertThrows(IllegalStateException.class, mpis::getParts);
- assertThat(error.getMessage(), is("CLOSING"));
+ Throwable error = assertThrows(IOException.class, mpis::getParts);
+ assertThat(error.getMessage(), is("ERROR"));
// There was no error with the cleanup.
assertNull(cleanupError.get());
@@ -586,8 +586,8 @@ public void testParseAfterCleanUp() throws Exception
mpis.deleteParts();
// The call to getParts should throw because we have already cleaned up the parts.
- Throwable error = assertThrows(IllegalStateException.class, mpis::getParts);
- assertThat(error.getMessage(), is("CLOSED"));
+ Throwable error = assertThrows(IOException.class, mpis::getParts);
+ assertThat(error.getMessage(), is("DELETED"));
// Even though we called getParts() we never even created the tmp directory as we had already called deleteParts().
assertFalse(_tmpDir.exists());