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
ocaml{c,opt} may truncate and recreate a .cmi, leading to (rare) failures of make -j #4991
Comments
Comment author: @alainfrisch OMake's approach is to force ocamlopt to use the .cmi file produced by ocamlc. This is done by creatin a dependency on this .cmi file (when then .mli file does not exist) for the ocamlopt rule, and to use "-intf-suffix .cmi" to let ocamlopt believe that the interface does exist. That said, we also observe random and rare cases of "corrupted compiled interface" during parallel builds under Windows. Maybe there is something wrong with the approach above, or it's just Windows that decides to do random errors. |
Comment author: letouzey :-) Thanks Alain, I actually though of this -intf-suffix, but I was so And indeed, I've also made .cmx depend on .cmi... Concerning the windows build, I played recently with mingw32 cross-compilation Best, |
Comment author: letouzey Ok, after testing Alain's tip I can confirm that "-intf-suffix .cmi" is Best regards, |
Comment author: @damiendoligez The way I see it, the best solution is to have a .mli for each .ml file. (For rather The next best thing would be to make the compiler output the compiled interface |
Comment author: @alainfrisch Damien: what's wrong with the -intf-suffix solution? |
Comment author: @xavierleroy Re: atomic file rename: it's unclear this can be done under Windows. The rename() function from the C standard library, interfaced in Caml as Sys.rename, fails if the destination file exists. The MoveFileEx Win32 system call can be cajoled into replacing an existing file, but it is not specified that this happens atomically. (Google "MoveFileEx atomic".) Re: opening the .cmi twice: the second open-for-reading is done to compute the MD5 digest of the data just written. OCaml's I/O channels don't support opening for reading and writing at the same time. Re: the -intf-suffix trick: I'm happy that it provides a workaround. Still, I have the feeling that something is slightly wrong here, so I'll leave this PR as a feature wish. |
Comment author: @damiendoligez I have no reason to believe there is currently something wrong with the -intf-suffix trick, but it still feels like a hack. |
Comment author: @gasche I'm hitting a new instance of this problem that I thought I would document here for posterity. The race condition happens not between ocamlc and ocamlopt, but two instances of ocamlopt invoking -pack to produce the same packed module, which does not have a corresponding .mli. The problem is the following: if the two runs try to produce the same .cmi file at precisely the same time, then one of them may make a mistake when computing the control sum (CRC) of the .cmi (which happens in between the beginning of writing the .cmi and before finishing the write), with some parts written or truncated by the other process taken into account. Both runs will then include the CRC they computed in the final .cmo, so it is likely that an incorrect CRC gets written in the .cmo file -- and that the .cmi produced is incorrect. It seems to me that, in that case, the workaround presented are not effective:
Reproductibility information: The race occurs about one times over three (so it's actually quite common) when running "make -j7" from the ocamlbuild subdirectory of the compiler (trunk@14532), after I first added a couple missing dependencies in the .depend that also broke compilation (ocamlbuild{_plugin,_unix_plugin,light} should depend on ocamlbuild_pack.cmi). |
Comment author: @gasche I fixed this instance of the problem by adding more dependencies (from the pack .cmo to the pack .cmi). I still think that, long-term, it would be nice to change the way we produce side-files (.cmi and .cmx) to a more atomic replace operation or, even better, replace_if_changed (we can look at the checksum of the existing file). |
Comment author: bobot Re: atomic file rename: why the atomicity guaranty of the rename is needed? It seems that the worse that can happen is to lose the compiled file .cm* during a power outage, but it is just a compiled file and discrepancy between .cm* will be detected. The second problem with non-atomicity is that the destination file can be missing during a period of time. But here the acceptable use case is when the destination file is the same than the temporary file .cmi. So we can test that the checksum is identical and just "touch" the destination file. In the other case, when the destination file is different, nobody should be opening the .cm* since they are not yet updated, it is an error of the build system. |
Comment author: @gasche Sorry for being unclear: a plain (not particularily atomic) "rename" would already be much more atomic than the way cmi files are currently generated, which is as follow:
Races between two processes writing partial information to the file happens quite often in this setting. A rename from an assumed-private temporary would not allow one compilation process to checksum a file that's been partially written by another, as it is currently possible. |
Comment author: bobot So even if, as Xavier remarked, "it's unclear [atomicity] can be done under Windows.", patches are welcomed for using an intermediary file during generation of files? Or patch that computes the checksum directly on the in memory data? |
Comment author: @gasche I would be interested in such patches, in any case. But be careful before investing your time: any downside of a new approach (eg. increased compilation time, or too-invasive change¹) would be a sufficient argument to maintain the statu quo, that works for most users (with high probability). ¹: eg. you do not want to re-implement part of the marshalling logic to compute checksum, as this would probably be quite fragile. Marshalling to a string instead of directly to a file would be a possible approach, but then we'd have to make sure that memory consumption isn't a problem (how large can .cmi files be? I heard that core.cmi is quite large; would we apply the same strategy on .cmo?). I think that write-in-temp than move is probably the easiest way forward in this case, but I'll let you decide if you're interested in working on this. |
Comment author: bobot The write-in-temp inside the compiler is a nice solution, but there is some choice to make. For example, when the cmi is identical should we still "touch" it? Another possibility can be to let the build system do the choice of the temporary file and do the test. The build system just need to be able to set the output filename for the cmi file using a command line option of ocamlc or ocamlopt, e.g. --outfile-cmi. This interface is simple and lets a lot of freedom to the user. One drawback is that Makefile/build tool must be modified for solving the problem of this bugreport. With a more general point of view, the possibility to set the filename of all the output file of the compiler (.cmt, .annot, ...) is a very nice feature to have. |
Comment author: bobot In fact -o does already the job, since every produced file filename as the same prefix than it (only the extension change) |
Comment author: bobot #78 will try to work on this possible solution:
|
Comment author: @xavierleroy Work in progress at #1307 |
PR merged, so closing this. |
Original bug ID: 4991
Reporter: letouzey
Status: acknowledged (set by @xavierleroy on 2010-04-18T12:31:37Z)
Resolution: open
Priority: normal
Severity: feature
Version: 3.11.2
Category: compiler driver
Related to: #5000 #5282 #7472
Monitored by: mfp mehdi @glondu bobot "Julien Signoles" @dra27 @dbuenzli @yakobowski
Bug description
When a .ml file has no corresponding .mli file, ocamlc and ocamlopt
create both the same .cmi file. During a parallel build that mixes
the byte and opt phases, this could lead to failure: imagine for instance
a process trying to read the .cmi (made earlier by ocamlc) at the precise
moment ocamlopt is recreating the .cmi, resulting in a "corrupted .cmi" error.
We have from time to time this kind of issue when building Coq
via make -j (unlimited number of process) on a quad-core.
Btw, a strace on ocamlc or ocamlopt shows that the .cmi is
first opened with O_WRONLY|O_CREAT|O_TRUNC, then a second time
with RDONLY. Is it done for some kind of verification ?
Would it be possible to modify ocaml{c,opt} in order to make them check
for an existing .cmi even if no .mli exists, and modify this .cmi only
if it differs from what the compiler intend to write ?
If this isn't easily doable, an option preventing the creation of
the .cmi would be great. In fact, there already exists a
Clflags.dont_write_files option, which is used precisely at the right
spot of Typemod. Could this flag be toggle someday by the user ?
For the moment, I'm circumventing this issue in our makefile by making sure
that ocamlc comes first and then doing something like
ocamlopt -c -o foo.tmp.cmx foo.ml &&
rm foo.tmp.cmi && mv foo.tmp.o foo.o && mv foo.tmp.cmx foo.cmx
It works, but this isn't exactly pretty.
Thanks in advance,
Pierre Letouzey
The text was updated successfully, but these errors were encountered: