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

Sandboxing on Windows does not work in presence of a virus scanner #11425

Open
MSoegtropIMC opened this issue Feb 1, 2025 · 3 comments · May be fixed by #11437
Open

Sandboxing on Windows does not work in presence of a virus scanner #11425

MSoegtropIMC opened this issue Feb 1, 2025 · 3 comments · May be fixed by #11437
Labels

Comments

@MSoegtropIMC
Copy link

Expected Behavior

Builds pass without issues

Actual Behavior

I see in various builds a lot errors of the form:

Error: failed to delete sandbox in
_build/.sandbox/027fb85dd9ad3f86a15ca2259967f6e0
Reason:
rmdir(_build/.sandbox/027fb85dd9ad3f86a15ca2259967f6e0\default\.ppx\f7b2eee21a8c0e54401ad28ca0581bfe): Directory not empty

this does not happen with all opam packages, but it happens very reliably with some, e.g. sel and coq-elpi. Interestingly they are both from the same author (@gares FYI). But I can't say there anything particularly strange about these packages. In all cases the remaining file in the sandbox is ppx.exe.

Reproduction

Build via opam e.g. the package sel on Windows when a virus scanner is present (I use GData, but it shouldn't matter much). It should fail > 80% with such an error.

The issue is that on Windows, unlike Linux and MacOS, files are identified by name and not by inode. On Linux a file name is just a link to an inode. If a a file is deleted, it doesn't delete the file, it just removes the inode link. If the inode link is 0, the file itself is deleted. An open file also has an inode link, so on Linux one can delete open files. On Windows this does not work. I file cannot be deleted while it is open.

The issue is that virus scanners tend to inspec all new .exe files, so they have this file opened, which means you cannot delete it. This is the reason why removing the sandbox fails on Windows if a virus scanner is present.

Specifications

  • Version of dune (output of dune --version): 3.16.1
  • Version of ocaml (output of ocamlc --version): 4.14.2
  • Operating system (distribution and version): Windows 11, cygwin, MinGW cross (Coq Platform)

Additional information

Possible remedies are:

  • retry deleting files if deleting a file fails - usually a virus scanner doesn't keep files locked for more than a few seconds
  • by default disable sandboxing on Windows
  • for cygwin MinGW cross dune it should be documented that the location of the config file is not cygwin home, but Windows home (somehow obvious but not to every body).
@MSoegtropIMC
Copy link
Author

I tried export DUNE_SANDBOX=none outside of opam, but this did not have an effect. I still get the same dune sandbox errors. Does opam filter out this environment variable? Should it have an effect?

@MSoegtropIMC
Copy link
Author

The 3 packages of Coq Platform which fail quite reliably (> 80%) unless I switch the virus checker completely off (not just advanced features) are:

sel
ppx_inline_test
coq-elpi

all other opam packages used by Coq Platform (about 180) compile reliably with virus checker enabled.

@maiste maiste added the windows label Feb 3, 2025
@MSoegtropIMC
Copy link
Author

I modified the windows unlink function here to retry 30x in 1s intervals in case of failure. This appears to work (and it does cost time only if it would have failed otherwise, but I am not sure if some error cases should be excluded or only some included). I will do a PR and we can discuss details in how to do this exactly there.

MSoegtropIMC added a commit to MSoegtropIMC/dune that referenced this issue Feb 3, 2025
MSoegtropIMC added a commit to MSoegtropIMC/dune that referenced this issue Feb 3, 2025
maiste pushed a commit to MSoegtropIMC/dune that referenced this issue Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants