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

caml_sys_open may block while holding runtime lock #5069

Closed
vicuna opened this Issue Jun 8, 2010 · 3 comments

Comments

Projects
None yet
1 participant
@vicuna
Copy link
Collaborator

vicuna commented Jun 8, 2010

Original bug ID: 5069
Reporter: @mmottl
Status: closed (set by @xavierleroy on 2011-05-12T15:15:12Z)
Resolution: fixed
Priority: normal
Severity: major
Version: 3.11.2
Fixed in version: 3.12.1+dev
Category: ~DO NOT USE (was: OCaml general)
Monitored by: dnuffer @Chris00 @mmottl

Bug description

caml_sys_open in byterun/sys.c uses fcntl to set the FD_CLOEXEC flag (if supported by that platform) on file descriptors after opening a file. Unfortunately, this happens outside of the OCaml runtime lock, which can lead to problems if fcntl blocks.

We have observed this issue when e.g. opening files on NFS. It seems glibc wraps the fcntl system call in a way that requires it to also call "stat", which is known to block under some circumstances.

Could you please change caml_sys_open to call fcntl in the "blocking section" used to open the file? - Thanks!

File attachments

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented May 10, 2011

Comment author: till

I failed to reproduce this bug and doubt that, as of glibc 2.13, fcntl calls stat while setting FD_CLOSEXEC. The kernel however has a small lock around the fd and it is possible that, on NFS, some operations hold that lock for a bit too long.
The patch to do the fcntl out of the ocaml lock is very simple and probably worth applying just to stray on the safe side.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented May 10, 2011

Comment author: @mmottl

The straces back then definitely showed that fcntl was calling "stat", though the problem might also be due to kernel locks. Hard to say, the problem was not reliably reproducible. Newer glibc versions may also behave differently. I agree we should fix this in any case to be on the safe side.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented May 12, 2011

Comment author: @xavierleroy

Ah, the mystery of the blocking fnctl()... The patch is sweet, though, so I applied it in the 3.12 branch. Will be part of 3.12.1.

@vicuna vicuna closed this May 12, 2011

@vicuna vicuna added the bug label Mar 20, 2019

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.