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

Ephemerons do not correctly handle Infix_tag #7810

Closed
vicuna opened this issue Jun 21, 2018 · 6 comments
Closed

Ephemerons do not correctly handle Infix_tag #7810

vicuna opened this issue Jun 21, 2018 · 6 comments
Labels

Comments

@vicuna
Copy link

vicuna commented Jun 21, 2018

Original bug ID: 7810
Reporter: @stedolan
Status: new
Resolution: open
Priority: normal
Severity: minor
Category: runtime system and C interface

Bug description

This function creates an ephemeron, fills it in, causes a GC and finally inspects the ephemeron. The ephemeron should not be cleared because the key is still live, since it is part of the return value.

let ephe x =
  let open Ephemeron.K1 in
  let e = create () in
  set_key e x;
  set_data e 42;
  Gc.full_major ();
  (x, get_data e)

The second component of this pair should never be None, e.g.:

# ephe (ref 1000);;
- : int ref * int option = ({contents = 1000}, Some 42)

However, it is None if the argument has Infix_tag (i.e. is a member of a set of mutually recursive functions other than the first):

# ephe (let rec f x = () and g x = () in g);;
Warning 26: unused variable f.
- : ('_a -> unit) * int option = (<fun>, None)

The bug is this check in minor_gc.c (which appears a couple of times):

if (Hd_val (*key) == 0){ /* Value copied to major heap */

If *key has Infix_tag, Hd_val (*key) can be nonzero even though it is copied to the major heap.

@vicuna
Copy link
Author

vicuna commented Jun 22, 2018

Comment author: bobot

Why in caml_oldify_one the test is if (hd == 0){ /* If already forwarded */? Are Infix_tag forwarded multiple times?

@vicuna
Copy link
Author

vicuna commented Jun 22, 2018

Comment author: bobot

For multicore, I remember that infix_tag handling could have changed? Is it true?

@vicuna
Copy link
Author

vicuna commented Jun 22, 2018

Comment author: @damiendoligez

In caml_oldify_one, if the tag is Infix_tag we call caml_oldify_one recursively on the real block (which can be forwarded only once) then patch up the returned pointer.

@vicuna
Copy link
Author

vicuna commented Jun 29, 2018

Comment author: @kayceesrk

Infix_tag handling remains the same in multicore.

@github-actions
Copy link

github-actions bot commented May 7, 2020

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@stedolan
Copy link
Contributor

Fixed by #9742

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

2 participants