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

wrong sharing of data between closures in bytecode #6119

Closed
vicuna opened this issue Aug 5, 2013 · 4 comments
Closed

wrong sharing of data between closures in bytecode #6119

vicuna opened this issue Aug 5, 2013 · 4 comments
Assignees
Milestone

Comments

@vicuna
Copy link

vicuna commented Aug 5, 2013

Original bug ID: 6119
Reporter: daweil
Assigned to: @alainfrisch
Status: closed (set by @xavierleroy on 2015-12-11T18:26:41Z)
Resolution: fixed
Priority: urgent
Severity: major
Version: 4.00.1
Target version: 4.01.1+dev
Fixed in version: 4.02.0+dev
Category: ~DO NOT USE (was: OCaml general)
Monitored by: @gasche

Bug description

When two functions share a reference, but are in two different files, the serialization of the two closures through the Marshal library is ok in native code but give a wrong execution in bytecode!!!

It seems that data sharing fails in bytecode, and that the virtual machine sees two separate references instead of one.

In the attached example, the first file defines:
let x = ref 0
let set = fun v -> x := v; printf "set x = %d@." !x

the second files defines:
let print = fun () -> printf "x = %d@." !x

and test the behavior of the two function before and afer marshalling :
type t = int ref * (int -> unit) * (unit -> unit)
let y:t = x, set, print
let test1 = print(); set 3; print ()

let test2 =
printf "after marshall/unmarshall@.";
let marshall = Marshal.to_string y [Marshal.Closures] in
let ((x, set, print) : t) = Marshal.from_string marshall 0 in
print (); set 42; print ()

Behavior of test1 and test2 should be identical. It is the case in native code, but not in bytecode, as if option Marshal.No_sharing was activated.

Steps to reproduce

  • unzip attached file
  • "ocamlbuild bugMarshalClosure.native--" to check that execution is correct
  • "ocamlbuild bugMarshalClosure.byte --" to see the bug : x = 3 instead of 42 when calling the unmarshalled set/print functions.

File attachments

@vicuna
Copy link
Author

vicuna commented Aug 6, 2013

Comment author: gerd

I could reproduce the bug. It seems to be essential that [print] and [x] are defined in distinct modules. An [x] defined in the same module is marshalled correctly.

Acutally, I'm not sure whether this is a bug, but it's certainly an inconsistency.

I used plain ocamlc to build in order to exclude any influence of ocamlbuild.

@vicuna
Copy link
Author

vicuna commented Oct 14, 2013

Comment author: hnrgrgr

Indeed, serialization of toplevel reference is unspecified. Changing this behaviour is probably a big change and I'm not sure it is worth.

What happens here, is that the closure representing 'set' in bytecode captures the references in its environment. While, the closure representing 'set' in native code and the 'print' closure in both byte- and native code make a direct access the global value in the BugClosure_lib module.

Hence, in bytecode, the unmarshalled 'x' value and 'set' function share the same reference (the marshalled one), while 'print' accesses the global value. In native code, the 'set' and 'print' access the global value, while the unmarshalled 'x' is copy of the global reference. Just add the following at the end of your example program to see the distinction:

printf "x = %d@." !BugClosure_lib.x;
printf "x = %d@." !x

Currently, in order to preserve consistency between bytecode and native code w.r.t to toplevel reference serialization, a workaround is to define toplevel references in a first module and define all functions using the reference on others modules. But this way, you will share the reference between the unmarshalled function and the original one.

If you really want to duplicate the reference, a workaround is to use local reference. That way you will always be consistent between byte- and native code:

let set, print =
let x = ref 0 in
let set = fun v -> x := v; printf "set x = %d@." !x
and print = fun () -> printf "x = %d@." !x in
set, print

To be certain that all the marshalled function share the same 'local' reference they all should be defined at once. Otherwise, I think the behaviour is also unspecified.

I'm not sure what to do to fix this bug report.

[Edit:typo]

@vicuna
Copy link
Author

vicuna commented Oct 14, 2013

Comment author: daweil

My wish would be that an error is raised in the Marshall library when such a case is encountered.
If it is not possible, at least, enhance the documentation of the Marshall library.

@vicuna
Copy link
Author

vicuna commented Apr 23, 2014

Comment author: @alainfrisch

The documentation for Marshal has been extended to explain this behavior (commit 14670 on trunk). Comments on the text are welcome.

@vicuna vicuna closed this as completed Dec 11, 2015
@vicuna vicuna added this to the 4.01.1 milestone Mar 14, 2019
@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
Projects
None yet
Development

No branches or pull requests

2 participants