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

Windows (mingw): Unix.set/clear_close_on_exec modifies the descriptor #4895

Closed
vicuna opened this issue Oct 16, 2009 · 3 comments
Closed

Windows (mingw): Unix.set/clear_close_on_exec modifies the descriptor #4895

vicuna opened this issue Oct 16, 2009 · 3 comments
Labels

Comments

@vicuna
Copy link

vicuna commented Oct 16, 2009

Original bug ID: 4895
Reporter: gerd
Status: closed (set by @xavierleroy on 2013-08-31T10:48:40Z)
Resolution: fixed
Priority: normal
Severity: minor
Version: 3.11.1
Fixed in version: 3.13.0+dev
Category: ~DO NOT USE (was: OCaml general)
Related to: #5416
Monitored by: @Chris00 "Christoph Bauer"

Bug description

The implementation of Unix.set/clear_close_on_exec creates a duplicate of the file handler, and changes the inheritance flag while doing so. The duplication changes the value of the descriptor, and is unnecessary, as Windows now supports changing the inheritance flag directly.

E.g.
let h = Hashtbl.create 10;;
Hashtbl.add h fd "Something";;
Unix.set_close_on_exec fd;;
Hashtbl.find h fd

raises Not_found

Additional information

use the win32 call SetHandleInformation

@vicuna
Copy link
Author

vicuna commented Oct 18, 2009

Comment author: @xavierleroy

SetHandleInformation is a relatively recent addition to Win32
(since Windows 2000 Professional / Server), so at the time this code was written there was probably no other option than duplicating the handle. I guess we can assume everyone has SetHandleInformation nowadays.

A (tested) patch would be most welcome.

@vicuna
Copy link
Author

vicuna commented Feb 27, 2012

Comment author: ripoche

Gerd's proposal matches what was implemented for bug #5416.
And indeed, I was unable to reproduce the issue with the patch applied.

(OCaml 3.12.1, Msvc7.1, Windows XP SP3 32bits)

@vicuna
Copy link
Author

vicuna commented Feb 27, 2012

Comment author: @xavierleroy

Very good. I'll mark this PR as resolved, then. Thanks for the notice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant