I've a got a weird situation happening that I'm hoping someone here might know how to resolve...
I have a Struts 2 page where users can download mp3 files. Downloading a file individually works a charm but I've noticed that when a user downloads multiple files at the same time, the data received is mangled - e.g., a downloaded mp3 file can contain parts of the other mp3 files!
The link to download each file invokes a "DownloadManager" Struts action and passes in the track ID as a parameter. The mechanism I use to send a file to the client is as follows:
FWIW, from a user perspective, when they click on a link they get a standard "Save File" dialog box, i.e., they are not forwarded away from the download page.
Anyone got any ideas on how I can make my download links thread-safe, so to speak, so that file data integrity is perserved?
You can't both download a file *and* an HTML page; a browser can only handle a single response.
Joined: Aug 12, 2010
Hi David, thanks for your comments.
Yes, you're right - it's not Struts 2, thanks for pointing that out.
On your other point - I don't actually want to download a file and an HTML page at the same time. The objective of the DownloadManager action is to perform a series of checks and then send the file to the client via HttpServletResponse, and this is what currently happens with great success. The forward mapping line at the end of the code is probably confusing matters but really it's of no consequence as it doesn't actually do anything (I get an "IllegalStateException: Cannot forward after response has been committed" in the logs).
I think my problem here is that * I believe * Struts v1 uses a singleton approach for Action classes. As such, all requests from the client are served by the same HttpServletResponse.
So, I think I have two options here to address the simultaneous requests problem:
1) Synchronize the code in question
2) Discard the Struts action class altogether and manage the download links via a standard Servlet instead - that way each request will spawn a new Servlet instance.
Any flaws in my thinking above or a better way to get around this problem?
Correct, there is a single action instance per mapping. But you're wrong about there only being a single HttpServletResponse--how would that make sense? That would screw up *every* Struts 1 application in the world. There's a new response passed in to the action for each request (just like there's a new request). As long as you're only using local variables and method parameters, you're fine on that count.
How come the file.read() always starts from offset 0? How come you bother with a random access file? Maybe you should check out any number of Struts 1 or just servlet file streamer examples available on the web; I think you're over-thinking this a bit.
Joined: Aug 12, 2010
David Newton wrote:How come you bother with a random access file?
The reason for that is my files can be up to 10 MB in size and the JVM just won't load large files into memory. My code above ensures that only 8k worth of data is loaded into the JVM at a time as the user downloads the file.
Back to my concurrency problem - I always understood that for every HttpServletRequest there's a corresponding HttpServletResponse. Is it not the case that if a user invokes a Struts action class twice concurrently, they will be invoking two separate requests and the container/framework will serve these two distinct requests with corresponding responses? In my case though, it looks like data from the second file download goes into the HttpServletResponse/ServletOutputStream of the first file download and the user winds up with a hybrids of both songs.
Again, if the user downloads one file at a time, all files are intact and everything is as expected.
I don't see anything that would lead me to believe you have a concurrency problem yet. Does the issue occur across browsers?
Joined: Aug 12, 2010
I meant to reply to this thread ages ago with the solution to my problem. I hope this helps anyone else who experiences a similar issue:
My *NOT* thread safe code looked like this:
To make the above code thread-safe and eliminate the garbled simultaneous downloads issue I reported, I just had to change my code as follows:
Moving the variables from within the class definition to inside the method made the code thread safe. When you define variables within the class definition, the variables belong to that class instance and retain their values between method calls. When you have a multi-threaded container where multiple requests are serviced by a same action class instance, you can get undesirable results.