Question: Why is MemoryFileUpload not released by DefaultHttpDataFactory?

[jameskleeh]

Expected behavior

I expect any MemoryFileUploads to be released when any of the cleanAll methods are called

Actual behavior

Mixed and Disk uploads are cleaned, but memory isn't. This seems intentional as the same logic is applied to attributes.

The upload/attributes are never added to the list that gets iterated over when cleaning

https://github.com/netty/netty/blob/4.1/codec-http/src/main/java/io/netty/handler/codec/http/multipart/DefaultHttpDataFactory.java#L245

Netty version

4.1.30

JVM version (e.g. java -version)

Java 8

OS version (e.g. uname -a)

macOS

[normanmaurer]

@jameskleeh sorry for the late response... This may be a bug. Are you interested in providing a PR ?

[jameskleeh]

@normanmaurer Sure thing

[jameskleeh]

@normanmaurer Done #9672

[fredericBregier]

@jameskleeh Indeed, that was intentional. Maybe wrongly, since you feel it should be included...
Your PR seems OK to change the behavior, but could you also fix the comments on the related "clean" methods, such that it does not explicitely stands for "disk" only (or mixed so mixed too) HttpData ?

[jameskleeh]

@fredericBregier Sure thing

[jameskleeh]

@fredericBregier I don't see what comments you are referring to. Can you point me in the right direction?

[fredericBregier]

Sure:

netty/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpDataFactory.java

Line 75 in c70254e

void cleanRequestHttpData(HttpRequest request);

netty/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpDataFactory.java

Line 80 in c70254e

void cleanAllHttpData();

netty/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpData.java

Line 121 in c70254e

void delete();

(even if this one was already wrong, since in AbstractMemoryHttpData, the delete method indeed call release on bytebuf)

I don't go everywhere, but I think this are the main ones.

Of course, we might also add a new method clean duplicate of :

netty/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostStandardRequestDecoder.java

Line 660 in c70254e

public void cleanFiles() {

since it now will delete all, even not files, in order to be clean (even if cleanFile will delete all). Or just changing the associated comment to ensure the new behavior is understood.
And same on

netty/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoder.java

Line 949 in c70254e

public void cleanFiles() {

[jameskleeh]

I've updated the Javadocs to what makes sense to me. I didn't add clean methods because I don't know your preferences on default methods for backwards compatibility and deprecation. That seems out of scope for this issue anyways.