OSC one off, maybe due to open following pending merge tag?

Author: xFrednet

Lib/test/test_regions/test_clean.py

@@ -28,8 +28,16 @@ def test_try_close_sub_region(self):
         # Cleaning a dirty parent region should clean the child as well
         region.clean()
 
-        # The region should now be clean
+        # The regions should now be clean
         self.assertFalse(sub.is_dirty)
+        self.assertEqual(sub._lrc, 1, "The local should be the only known reference")
+        self.assertEqual(sub._osc, 0, "No subregions should be present")
+        self.assertEqual(region._osc, 1)
+
+        # Removing the reference into sub, should close it and inform the parent
+        sub = None
+        self.assertEqual(region._osc, 0)
+
 
     def test_clean_removes_unreachable(self):
         region = Region()

Lib/test/test_regions/test_core.py

@@ -47,6 +47,21 @@ def test_instance_dict_is_owned(self):
         self.assertTrue(r.owns(r.__dict__))
 
 
+class TestRegionCounts(unittest.TestCase):
+    def test_osc_1(self):
+        r = Region()
+        r.sub = Region()
+
+        # Pre-condition
+        self.assertEqual(r._osc, 0, "The sub region should be closed")
+
+        # This should open the sub-region
+        sub = r.sub
+
+        # Post-condition
+        self.assertEqual(r._osc, 1, "The sub region should be open now")
+
+
 class ImplicitFreezingForImmortal(unittest.TestCase):
     def test_implicit_freeze_importal(self):
         # This would ideally check that the immortal objects

Python/region.c

@@ -862,6 +862,8 @@ static int regiondata_set_parent(Py_region_t region, Py_region_t new_parent) {
             return 1;
         }
 
+        // TODO(regions): xFrednet: This is probably wrong? At least cleaning
+        // (a region and sub region) seems to have a off-by-one counting a OSC of 2
         if (regiondata_dec_osc(old_parent)) {
             // Undo the inc_osc from above.
             regiondata_dec_osc(new_parent);
@@ -1462,7 +1464,6 @@ int regiondata_clean(PyObject* bridge) {
         Py_region_t item_region = _PyRegion_Get(item);
 
         assert(regiondata_is_bridge(item_region, item));
-        // TODO: Account in LRC for reference from owner, if present.
 
         // Store metadata for the new region
         assert(HAS_DATA(item_region));
@@ -1491,17 +1492,16 @@ int regiondata_clean(PyObject* bridge) {
         }
         staged_ref_commit(staged_ref);
 
+        // TODO: Account in LRC for reference from owner, if present.
+
+
         // TODO(regions): xFrednet: This doesn't account for region union...
         //
-        // `stage_objects` accounts for a reference from a contained object to
-        // the added object, mening that the LRC is missing a count of 1 here.
-        // We increment the LRC if it doesn't have a owner.
-        if (owner == 0) {
-            SUCCEEDS(regiondata_inc_lrc(clean_region));
-            // FIXME(regions): xFrednet: The hack currently marks the region as
-            // dirty when the LRC is increased. the following function should
-            // no longer be used when all barriers are in place
-            SUCCEEDS(regiondata_mark_as_clean(clean_region));
+        // `stage_objects` doesn't know about the reference from the owner and
+        // counts it as a local reference. Meaning the LRC counts one more
+        // reference than present.
+        if (owner != 0) {
+            SUCCEEDS(regiondata_dec_lrc(clean_region));
         }
 
         // The region should now be marked as clean