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

Warp sends truncated files when using sendfile #371

Closed
reiddraper opened this Issue May 1, 2015 · 9 comments

Comments

Projects
None yet
3 participants
@reiddraper
Copy link
Contributor

commented May 1, 2015

Tested with warp 3.0.5.2 and 3.0.12.1, and on Linux and Mac OSX.

Sample Main.hs.

module Main where

import Network.Wai
import Network.Wai.Handler.Warp
import Network.HTTP.Types

main :: IO ()
main = run 3000 $ \_ rsp -> rsp $ responseFile status200 [] "foo" Nothing

Write to the file foo:

$ echo foobarbaz > foo
$ ./Main.hs

$ curl -v localhost:3000/
<snip>
Content-Length: 10
<snip>
foobarbaz

Everything looks good. Now edit foo, while leaving the server running:

$ echo foobarbaz >> foo

$ curl -v localhost:3000/
<snip>
Content-Length: 20
<snip>
foobarbaz

Curl hangs, and eventually times out. This little experiment appears to be non-determinstic, as well. Disabling send-file when running the test seems to never exhibit the bug (-f -allow-sendfilefd). My hunch is there is some timing issue with re-using file descriptors to pass to sendfile. You may have more luck reproducing this by significantly changing the file size between the first and second curl.

@snoyberg

This comment has been minimized.

Copy link
Member

commented May 1, 2015

This looks like the file descriptor cache, pinging @kazu-yamamoto. Can you try disabling that cache and see if it resolves your problem?

@reiddraper

This comment has been minimized.

Copy link
Contributor Author

commented May 1, 2015

I have not yet been able to reproduce the issue with a setFdCacheDuration of 0. The apache documentation seems to suggest this is a known issue with caching file-descriptors and using sendfile:

This file handle caching is done once at server start or restart, only. So whenever one of the cached files changes on the filesystem you have to restart the server (see the Stopping and Restarting documentation). To reiterate that point: if the files are modified in place without restarting the server you may end up serving requests that are completely bogus. You should update files by unlinking the old copy and putting a new copy in place. Most tools such as rdist and mv do this.

@snoyberg

This comment has been minimized.

Copy link
Member

commented May 1, 2015

In that case, should we just update the documentation and consider this working as expected? Would you be interested in sending a PR for an appropriate doc fix (i.e., somewhere that may have helped you)?

@kazu-yamamoto

This comment has been minimized.

Copy link
Contributor

commented May 1, 2015

This is a common problem of caching. We should clarify doc or change the default value to 0.

@reiddraper

This comment has been minimized.

Copy link
Contributor Author

commented May 1, 2015

Setting the default to 0 (no file descriptor cache) seems like a sensible compromise between performance (still using sendfile) and correctness. In production, with truly static assets, users can crank this up to 10 or 30 seconds.

@snoyberg

This comment has been minimized.

Copy link
Member

commented May 3, 2015

Kazu: since the file descriptor code is yours, do you want to make a
decision on this?

On Fri, May 1, 2015, 5:58 PM Reid Draper notifications@github.com wrote:

Setting the default to 0 (no file descriptor cache) seems like a sensible
compromise between performance (still using sendfile) and correctness. In
production, with truly static assets, users can crank this up to 10 or 30
seconds.


Reply to this email directly or view it on GitHub
#371 (comment).

@kazu-yamamoto

This comment has been minimized.

Copy link
Contributor

commented May 4, 2015

Let's set the default to 0 and clarify the issue and write a recommended value (eg 10 seconds) for productions in the doc.

Since it is in the golden week (a long national holidays) in Japan, I cannot take time. @snoyberg Would you take care of this?

snoyberg added a commit that referenced this issue May 4, 2015

@snoyberg

This comment has been minimized.

Copy link
Member

commented May 4, 2015

I've made the modification in de64974. Can you guys review to make sure the text is clear?

@snoyberg

This comment has been minimized.

Copy link
Member

commented May 5, 2015

Thanks for the review guys, code is now on Hackage. Closing.

@snoyberg snoyberg closed this May 5, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.