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

net: Add ability to disable sendfile #9694

Closed
anthonyfok opened this issue Jan 26, 2015 · 15 comments
Closed

net: Add ability to disable sendfile #9694

anthonyfok opened this issue Jan 26, 2015 · 15 comments

Comments

@anthonyfok
Copy link

Hello,

An issue "Cannot see updated files from public/ in VirtualBox shared folders" gohugoio/hugo#841 has been reported to the Hugo project. @itoed, the original reporter, has correctly deduced the culprit as a long-time bug in VirtualBox with its sendfile() support, which both Apache and Nginx have provided workarounds for by allowing the end user to disable the use of sendfile().

I think it affects other Go web applications running inside VirtualBox too, so it would appear to me that the best place to fix this is in ReadFrom() inside src/net/tcpsock_posix.go, perhaps by making the use of OS-optimized sendfile() a user-configurable option?

Many thanks for your kind consideration!

@mikioh mikioh changed the title net: Add ability to disable sendfile() so e.g. net/http can serve properly serve updated files inside Vagrant and VirtualBox net/http: Add ability to disable sendfile Jan 26, 2015
@mikioh mikioh changed the title net/http: Add ability to disable sendfile net: Add ability to disable sendfile Jan 26, 2015
@bradfitz
Copy link
Contributor

I don't like adding unnecessary configuration at all, especially unnecessary configuration to work around bugs.

If anything, we could auto-detect VirtualBox in the net package and never use sendfile. But no new option.

What is the cheapest way to detect whether we're running in a VirtualBox environment?

@itoed
Copy link

itoed commented Jan 26, 2015

Hi Brad,

Yes, it's definitely painful having to work around bugs in other projects. Sadly, there doesn't seem to have been any progress on the VirtualBox issue in four years.

I like your idea, however I imagine it wouldn't solve the problem for boot2docker. If the application using sendfile was running within a Docker container via VirtualBox, I'm not sure you could detect that shared folders in VirtualBox were being used.

Unfortunately I don't have a better suggestion. Nginx and Apache do offer the option to disable sendfile, so I'm wondering if there could be another reason for them to provide it.

Would it be out of consideration for golang to do the same? I believe it's pretty normal to use shared folders with boot2docker, and a matter of time before other go applications run into this issue.

Unless, of course, there is a way to detect if Docker is running within VirtualBox shared folders.

@bradfitz
Copy link
Contributor

@itoed, no new API surface for this. No configuration. No knobs. I don't care that other projects have more knobs. Go is great because it doesn't.

I'm not proposing detecting shared folders. I'm proposing detecting running inside Virtual Box at all and disabling transparent sendfile entirely in that case, for any files.

@anthonyfok
Copy link
Author

Hi Brad,

Great idea about detecting if we are inside VirtualBox (in lieu of yet another option)! That is truly thinking out of the box that makes Go great!

I came across this (white/grey/black hat?) article: http://spth.virii.lu/eof2/articles/WarGame/vboxdetect.html, but then realize it may be dependent on the guest OS and a bit too complicated.

However, since we know that we want to disable sendfile() if and only if sendfile() fails to do its work, perhaps we can use sendfile() itself to test for this errant behaviour?

I have just done a quick test, i.e. after running hugo --watch server and edited content/about.md, changes are instantly visible if I do cat public/about/index.html from the shell, but running curl http://localhost:1313/about/ gets the same old content unchanged, even if I were to restart the Hugo server! The only way to remedy this is to rm -rf public/ and restart hugo --watch server.

So yes, I think it will work! :-)

@c4milo
Copy link
Member

c4milo commented Jan 26, 2015

I think it is cleaner to allow disabling sendfile() than adding code to detect whether the server is running on VirtualBox or not. Disabling sendfile() seems to be a common feature in most HTTP servers. sendfile() doesn't even work well in OS X. (@bradfitz is right)

my 2 cents.

@bradfitz
Copy link
Contributor

Again, I don't want to add new options.

sendfile works on OS X.

@c4milo
Copy link
Member

c4milo commented Jan 26, 2015

@anthonyfok I'm now leaning more towards @bradfitz's side. Specially since it seems you can work around the issue in Virtualbox by using NFS instead: http://docs-v1.vagrantup.com/v1/docs/nfs.html

@simonkern
Copy link

According to this:
http://repo.hackerzvoice.net/depot_ouah/Red_%20Pill.html

There is a quite simple way to detect whether we're running inside a VirtualBox env. Didn't have the chance to test it though.

@c4milo
Copy link
Member

c4milo commented Jan 26, 2015

There is nothing to be fixed in Go, IMHO. This is clearly a VirtualBox issue with a workaround through NFS shared folders.

@minux
Copy link
Member

minux commented Jan 26, 2015

I agree. We shouldn't workaround buggy file system implementations.

Detecting VM is not going to be easy as VM will constantly try to thwart
such detection techniques (most such techniques rely on imperfect
emulation, that is, a bug in the VMM). Let's don't go down that road.

@minux minux closed this as completed Jan 26, 2015
@anthonyfok
Copy link
Author

@minux 马先生您好: I think we should give @bradfitz's suggestion an honest effort instead of just closing this issue before this discussion is over.

Please considering re-opening this issue. At least give us contributors a chance to think it through and try to contribute a patch. We can then discuss the merit of the patch (e.g. one that detects a working sendfile() rather than merely detecting whether we are inside VirtualBox), and decide whether we want to use it or not, and only then we close this issue.

@minux
Copy link
Member

minux commented Jan 27, 2015

How could you detect a working sendfile? The problem is not that sendfile
doesn't
work at all inside VirtualBox, but it doesn't work properly on files from
shared folder.

We don't know where the shared folder is, and even if we can find out (by
looking
at mounted file systems, for example), we shouldn't blindly trying to read
some file
in some random folder without users' explicit request. (Even reading the
list of
mounted file systems might be undesirable for some users.)

There might be other broken file system implementations, and I don't think
we
should test for all of them.

@slimsag
Copy link

slimsag commented Jan 27, 2015

I agree with @minux.

A workaround for a broken/buggy FS shouldn't be implemented in Go: the broken virtualbox FS should be fixed.

@anthonyfok
Copy link
Author

Hi @minux,

How could you detect a working sendfile? The problem is not that sendfile doesn't work at all inside VirtualBox, but it doesn't work properly on files from shared folder.

Thank you for your detailed explanation. I have indeed missed the part about this bug manifesting itself only when inside a VirtualBox shared folder, and it would be unfeasible and even dubious if Go were to try to detect that.

@slimsag, You're right, this is really a VirtualBox bug and should be fixed in VirtualBox.

Thank you all for an illuminating lesson.

@wader
Copy link

wader commented Oct 21, 2015

I did a workaround that uses seccomp to disable sendfile that i use while developing:
https://github.com/wader/disable_sendfile_vbox_linux

@golang golang locked and limited conversation to collaborators Dec 29, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants