Mantis Bug Tracker
Main | My View | View Issues | Change Log | Roadmap
  

View Issue Details [ Jump to Notes ] [ Issue History ] [ Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0005340OCamlOCaml generalpublic2011-08-16 15:482012-03-15 09:58
Reporterdb 
Assigned To 
PrioritynormalSeverityminorReproducibilityalways
StatusresolvedResolutionno change required 
PlatformOSOS Version
Product Version3.12.1 
Target VersionFixed in Version 
Summary0005340: Issue 0005258: the patch is incorrect
Description(I cannot reopen the issue so I have to create another one)

The original reporter decided that

let () =
  let state = ref 0 in
  while true do
    Printf.printf "Running loop #%d\n%!" !state;
    Thread.delay 0.1;
    let fd = Unix.openfile "test" [Unix.O_RDONLY] 0o640 in
    Printf.printf "Opened file\n%!";
 
    (* This function implicitely creates
     * a C runtime descriptor.
     * Uncommenting it leads to "Too many
     * open file" after (here) 2046 iterations. *)
    (* ignore(Unix.in_channel_of_descr fd); *)

    Unix.close fd;
    Printf.printf "Closed file\n%!";
    incr state
  done

is a problem, and created a fix. IMHO, there is no bug here; once
you constructed a channel from a file descriptor, the descriptor is owned by
the channel, and you only have to close the latter:

    let fd = Unix.openfile "test" [Unix.O_RDONLY] 0o640 in
    let ch = Unix.in_channel_of_descr fd) in
    close_in ch;

So fixing a non-existent issue created the new one: OCaml debugger under Win32 is broken now (and probably many other libraries that use Unix.in_channel_of_descr/Unix.out_channel_of_descr). It's a pity that an untested patch went to the release so easily (rev 11030 in the OCaml repository).

That said, there still is a real descriptor leak in Unix.fstat:

    let fd = Unix.openfile "test" [Unix.O_RDONLY] 0o640 in
    ignore(Unix.fstat fd);
    Unix.close fd;

Probably the proposed

    CRT_fd_val(handle) = fd;

will be OK there.
TagsNo tags attached.
Attached Files

- Relationships

-  Notes
(0006098)
doligez (manager)
2011-08-23 17:07

 > is a problem, and created a fix. IMHO, there is no bug here; once

There is at least a discrepancy: under Unix, the file descriptor is shared by
all the channels opened on it. This is what the patch tries to fix.

> you constructed a channel from a file descriptor, the descriptor is owned by
> the channel, and you only have to close the latter:

There is something I don't understand, here. As far as I can tell, this is still
true after applying the patch.

> OCaml debugger under Win32 is broken now

Could you give more details ? Why is it broken ? I'd like to see if we can get
a fix that satisfies everyone.
(0006099)
db (reporter)
2011-08-23 19:56

 Now after

    let fd = Unix.openfile "test" [Unix.O_RDONLY] 0o640 in
    let ch = Unix.in_channel_of_descr fd) in
    Unix.close fd;
    (...)

(Unix.close fd) also closes the underlying channel's file descriptor, and all subsequent channel I/O operations will fail. It was not the case before. Is it normal behavior under Unix? I assume that's why OCaml debugger could not connect to debugged process. I did not do much testing: just reverted the patch and the debugger started to work again. But I promise to provide more details in the near future.
(0006100)
xleroy (administrator)
2011-08-24 09:14

 Hello Dmitry,

Yes, what you describe is normal behavior under Unix: the in_channel "ch" shares the same OS file descriptor as the Unix.filedescr "fd".

I can't find documentation on _open_ofshandle() that says whether it duplicates the given HANDLE or just uses it as is. The former would explain some of your observations, maybe not all.

Thanks for any light you can shed here.
(0006101)
ripoche (reporter)
2011-08-24 11:19
edited on: 2011-08-24 11:33

 I just wanted to point out that the patch from bug 5258 seems to fix a rather annoying OMake issue.

Without the patch, OMake freezes randomly on Windows in the middle of builds. You then have to restart it manually.

I have no absolute certainty here, but I couldn't get it to freeze with the patch. Previously, every other build triggered the issue.

I've been using this version for just a couple of days, it is too early for conclusions.

For reference, that's OMake 0.9.8.6 (RC1) built with OCaml 3.11.2 + patch of bug 5258.
(0006108)
ripoche (reporter)
2011-08-30 15:23

 The OMake issue I was talking about is
http://bugzilla.metaprl.org/cgi-bin/show_bug.cgi?id=708 [^]
(0006169)
ripoche (reporter)
2011-10-17 17:04

 Although I kept the OCaml patch, the OMake freezes turned out to be something else.

Sorry for the noise.
(0007086)
xleroy (administrator)
2012-03-15 09:58

 In the absence of new information, I'm setting this PR to "no change required". As far as I can see, the currently-implemented behavior is consistent between Win32 and Unix.

- Issue History
Date Modified Username Field Change
2011-08-16 15:48 db New Issue
2011-08-23 17:07 doligez Note Added: 0006098
2011-08-23 17:07 doligez Status new => feedback
2011-08-23 19:56 db Note Added: 0006099
2011-08-24 09:14 xleroy Note Added: 0006100
2011-08-24 11:19 ripoche Note Added: 0006101
2011-08-24 11:20 ripoche Note Edited: 0006101
2011-08-24 11:21 ripoche Note Edited: 0006101
2011-08-24 11:23 ripoche Note Edited: 0006101
2011-08-24 11:33 ripoche Note Edited: 0006101
2011-08-30 15:23 ripoche Note Added: 0006108
2011-10-17 17:04 ripoche Note Added: 0006169
2012-03-15 09:58 xleroy Note Added: 0007086
2012-03-15 09:58 xleroy Severity major => minor
2012-03-15 09:58 xleroy Status feedback => resolved
2012-03-15 09:58 xleroy Resolution open => no change required

Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker