Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0004704OCaml-OCaml generalpublic2009-01-29 11:032009-04-19 10:58
Assigned Toxleroy 
PlatformOSOS Version
Product Version3.11.0 
Target VersionFixed in Version3.11.1+dev 
Summary0004704: Bug in generational global roots handling
DescriptionI believe the new generational global roots handling has a bug as follows. Suppose we initialize a variable to Val_unit and register its address as a generational global root. Nothing happens during the registration process because Is_block(Val_unit) does not hold. Suppose we then modify the contents of the root, using caml_modify_generational_global_root, to a value v such that Is_block(v) does hold. Unfortunately, since Is_block(oldval) doesn't hold in caml_modify_generational_global_root, the root isn't actually added to either the young or the old root lists as it should be. The root therefore never gets scanned, and a crash follows in due course.

I've been testing the patch below (against 3.11.0) which seems to work. I think the first new conditional is straightforward enough. The second is tricky though in the case where the root contained a boxed young value and is now being modified to contain an unboxed one. We could just leave the young global roots list alone in this case and wait for the next minor GC sweep to move the young roots onto the old roots list (at which point unboxed ones will be dropped). However, I think it better to explicitly remove it from the young list straight away, as the code does here. It avoids any possibility of subtle interactions with the user's expectations as to the root actually having been removed immediately following any subsequent call to the root-removal function after this boxed->unboxed modification. I can't actually spot any case in which there would actually be a crash if you don't remove it immediately, but some wierd stuff can certainly happen (for example if the user does "boxed->unboxed modify, remove root, directly set root contents to a non-young boxed value, insert root" then I think we could actually end up with the root on both lists).

--- byterun/globroots.c Thu Jan 08 16:32:00 2009 +0900
+++ byterun/globroots.c Thu Jan 29 09:42:45 2009 +0000
@@ -232,6 +232,26 @@
     caml_delete_global_root(&caml_global_roots_old, r);
     caml_insert_global_root(&caml_global_roots_young, r);
+ else if (!Is_block(oldval) && Is_block(newval)) {
+ /* The previous value in the root was unboxed but now it is boxed.
+ The root won't appear in any of the root lists thus far (by virtue
+ of the operation of [caml_register_generational_global_root]), so we
+ need to make sure it gets in, or else it will never be scanned. */
+ if (Is_young(newval))
+ caml_insert_global_root(&caml_global_roots_young, r);
+ else if (Is_in_heap(newval))
+ caml_insert_global_root(&caml_global_roots_old, r);
+ }
+ else if (Is_block(oldval) && !Is_block(newval)) {
+ /* The previous value in the root was boxed but now it is unboxed, so
+ the root should be removed. If [oldval] is young, this will happen
+ anyway at the next minor collection, but it is safer to delete it
+ here. */
+ if (Is_young(oldval))
+ caml_delete_global_root(&caml_global_roots_young, r);
+ else if (Is_in_heap(oldval))
+ caml_delete_global_root(&caml_global_roots_old, r);
+ }
   *r = newval;
TagsNo tags attached.
Attached Files

- Relationships

-  Notes
xleroy (administrator)
2009-03-28 16:27

Thanks for tracking down this bug and for preparing a patch. It was merged in the 3.11 release branch and will go in 3.11.1.

- Issue History
Date Modified Username Field Change
2009-01-29 11:03 shinwell New Issue
2009-02-09 17:50 xleroy Status new => assigned
2009-02-09 17:50 xleroy Assigned To => xleroy
2009-03-28 16:27 xleroy Note Added: 0004867
2009-03-28 16:27 xleroy Status assigned => closed
2009-03-28 16:27 xleroy Resolution open => fixed
2009-04-19 10:58 xleroy Fixed in Version => 3.11.1+dev
2017-02-23 16:36 doligez Category OCaml general => -OCaml general

Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker