Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebFlux temporary file not always deleted with parallel uploads. #31217

Closed
davefarrelly opened this issue Sep 13, 2023 · 11 comments
Closed

WebFlux temporary file not always deleted with parallel uploads. #31217

davefarrelly opened this issue Sep 13, 2023 · 11 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches status: feedback-provided Feedback has been provided type: bug A general bug
Milestone

Comments

@davefarrelly
Copy link

davefarrelly commented Sep 13, 2023

I am working on a project using spring-webflux:6.0.8 and I have noticed an issue where temporary files created during a FilePart upload are not always getting deleted when uploads are done in parallel.

However, if I do slow sequential uploads, the temporary files are getting deleted consistently as expected.

I have a reproducer with a single endpoint that receives a FilePart. Sending ~30 requests in parallel with with 28MB file seems to reproduce the issue consistently.

spring-tempfile-reproducer.zip

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 13, 2023
@poutsma poutsma self-assigned this Oct 12, 2023
@poutsma poutsma added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Oct 13, 2023
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Oct 31, 2023

I'm not able to reproduce the issue. There may be something more specific here that depends on how the client creates and uses connections. Can you share the client code as well?

This is what I used:

public class ClientApp {

    public static void main(String[] args) {
        FileSystemResource resource = new FileSystemResource("..."); // 30 MB file
        WebClient client = WebClient.create();
        Flux.range(1, 50)
                .flatMap(aLong -> client.post()
                        .uri("http://localhost:9890/upload")
                        .body(BodyInserters.fromMultipartData("uploadedFile", resource))
                        .retrieve()
                        .toBodilessEntity())
                .blockLast();
    }

}

@rstoyanchev
Copy link
Contributor

Note that it's worth trying with 6.0.10 where there was a related change that could have an impact on this, see c1fe571.

@rstoyanchev rstoyanchev added the status: waiting-for-feedback We need additional information before we can continue label Oct 31, 2023
@davefarrelly
Copy link
Author

@rstoyanchev thanks for the response!

I was actually using JMeter with the "bzm - Parallel Controller" plugin to make the requests.

I will test against our real application with a client similar to what you provided and also try 6.0.10 and get back to you with the results.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Oct 31, 2023
@davefarrelly
Copy link
Author

@rstoyanchev quick update, I tried updating to 6.0.10 and surprisingly none of the temporary files are getting deleted on this version. (Note. this is still using JMeter). Is there any config changes or anything needed with this version?

@davefarrelly
Copy link
Author

Tested using the client code provided instead of JMeter and I am still seeing the same results:

6.0.8: Couple of temporary files remain on disk
6.0.10: All 50 temporary files remain on disk

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Oct 31, 2023

Thanks for the additional info. I should have mentioned, might as well use the latest 6.0.13 (instead of 6.0.10), just in case. Also if you get a chance to run my sample code above, to confirm whether it reproduces it for you or not.

@davefarrelly
Copy link
Author

Yeah I tested 6.0.13 and seems to be behaving the same as 6.0.10, none of the files are getting deleted.

I ran the sample you gave above and results were the same on my end 🤔

@davefarrelly
Copy link
Author

Hi @rstoyanchev @poutsma I have tried updating to Spring Boot 3.2.0 released today (Spring Framework 6.1.1) and I am still seeing this issue. Where a couple of files (~2 out of 50) are not getting cleaned up.

image

@poutsma
Copy link
Contributor

poutsma commented Nov 30, 2023

Hi @davefarrelly, we are aware that the issue remains in 6.1.1, and are still investigating.

@jhoeller jhoeller added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 30, 2024
@jhoeller jhoeller added this to the 6.1.x milestone Jan 30, 2024
@AlexejTimonin
Copy link
Contributor

AlexejTimonin commented Mar 10, 2024

A tip to anyone experiencing temp folder filling up with zombie temp files due to this issue. Make sure to call delete on the FilePart when you're done working with it.

Given this cURL from 1 terminal session (You could call it MacGyver load testing), the file is 280kb curl manual:

while true; do curl -v -F uploadedFile=@curl-man.txt http://localhost:8080/upload; done

And this controller:

    @PostMapping
    public Mono<String> uploadFile(@RequestPart("uploadedFile") FilePart uploadedFile) {
        var newFile = Path.of("c:/test", UUID.randomUUID().toString());
        return uploadedFile.transferTo(newFile)
                .then(Mono.just(uploadedFile.filename()));
    }

The Temp folder will start to fill up with zombie temp files which are not deleted after about 30 seconds of curl spam:
image

To fix it, call delete on the FilePart when you are done:

    @PostMapping
    public Mono<String> uploadFile(@RequestPart("uploadedFile") FilePart uploadedFile) {
        var newFile = Path.of("c:/test", UUID.randomUUID().toString());
        return uploadedFile.transferTo(newFile)
                .then(uploadedFile.delete())
                .then(Mono.just(uploadedFile.filename()));
    }

With the controller above, I've ran curl from 2 sessions for 1 minute with no zombie Temp files detected.

Edit: Version - 6.1.4

qeeqez added a commit to qeeqez/spring-framework that referenced this issue Mar 18, 2024
@simonbasle simonbasle self-assigned this Apr 10, 2024
@simonbasle simonbasle added the for: backport-to-6.0.x Marks an issue as a candidate for backport to 6.0.x label Apr 15, 2024
@github-actions github-actions bot added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-6.0.x Marks an issue as a candidate for backport to 6.0.x labels Apr 15, 2024
simonbasle added a commit that referenced this issue Apr 15, 2024
Before this change temporary files would not consistently be deleted
when the connection which uploads the multipart files closes naturally.

This change uses the usingWhen Reactor operator to ensure that the
termination of the connection doesn't prevent individual file parts
from being deleted due to a cancellation signal.

See gh-31217
Closes gh-32638
@simonbasle simonbasle modified the milestones: 6.1.x, 6.1.7 Apr 15, 2024
@simonbasle
Copy link
Contributor

@davefarrelly I think this has finally been resolved. The fix should be available soon in a snapshot and I've also backported it to the 6.0.x branch.

If you have an opportunity to test out the snapshot, let us know. thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches status: feedback-provided Feedback has been provided type: bug A general bug
Projects
None yet
Development

No branches or pull requests

7 participants