Issue 4383 - changes from review

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
This commit is contained in:
Lachlan Roberts 2020-01-24 09:49:00 +11:00
parent f9f2ccaefa
commit f76687ad68
2 changed files with 18 additions and 12 deletions

View File

@ -56,10 +56,16 @@ import org.eclipse.jetty.util.log.Logger;
* <p> * <p>
* Deleting the parts can be done from a different thread if the parts are parsed asynchronously. * 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. * 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. * is done by the parsing thread.
* </p> * </p>
* <pre>{@code * <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()
* +--------------------------------------------------------------+ * +--------------------------------------------------------------+
* | | * | |
@ -67,7 +73,7 @@ import org.eclipse.jetty.util.log.Logger;
* UNPARSED -------> PARSING --------> PARSED ------------------>DELETED * UNPARSED -------> PARSING --------> PARSED ------------------>DELETED
* | ^ * | ^
* | | * | |
* +----------------> ERROR ---------------------+ * +---------------> DELETING -------------------+
* deleteParts() parsing thread * deleteParts() parsing thread
* }</pre> * }</pre>
* @see <a href="https://tools.ietf.org/html/rfc7578">https://tools.ietf.org/html/rfc7578</a> * @see <a href="https://tools.ietf.org/html/rfc7578">https://tools.ietf.org/html/rfc7578</a>
@ -79,8 +85,8 @@ public class MultiPartFormInputStream
UNPARSED, UNPARSED,
PARSING, PARSING,
PARSED, PARSED,
DELETED, DELETING,
ERROR DELETED
} }
private static final Logger LOG = Log.getLogger(MultiPartFormInputStream.class); private static final Logger LOG = Log.getLogger(MultiPartFormInputStream.class);
@ -406,11 +412,11 @@ public class MultiPartFormInputStream
switch (state) switch (state)
{ {
case DELETED: case DELETED:
case ERROR: case DELETING:
return; return;
case PARSING: case PARSING:
state = State.ERROR; state = State.DELETING;
return; return;
case UNPARSED: case UNPARSED:
@ -423,10 +429,10 @@ public class MultiPartFormInputStream
} }
} }
uncheckedDeleteParts(); delete();
} }
private void uncheckedDeleteParts() private void delete()
{ {
MultiException err = null; MultiException err = null;
for (List<Part> parts : _parts.values()) for (List<Part> parts : _parts.values())
@ -631,7 +637,7 @@ public class MultiPartFormInputStream
state = State.PARSED; state = State.PARSED;
break; break;
case ERROR: case DELETING:
state = State.DELETED; state = State.DELETED;
cleanup = true; cleanup = true;
break; break;
@ -642,7 +648,7 @@ public class MultiPartFormInputStream
} }
if (cleanup) if (cleanup)
uncheckedDeleteParts(); delete();
} }
} }

View File

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