3202 - bugfix: 'start-running' and refcounts

When you pass an ingredient to a recipe using 'start-running' it mostly
behaves identically to performing a regular function call. However, if
the calling function completed before the new routine had a chance to
run, the ingredients passed in ran the risk of being reclaimed.

In response, let's always increment refcounts at the time of a function
call rather than when the ingredients are read inside the callee.

Now the summary of commit 3197 is modified to this:

  Update refcounts of products after every instruction, EXCEPT:

    a) when instruction is a non-primitive and the callee starts with
    'local-scope' (because it's already not decremented in 'return')

  OR:

    b) when instruction is primitive 'next-ingredient' or
    'next-ingredient-without-typechecking'

5 files changed, 75 insertions, 7 deletions

diff --git a/027call_ingredient.cc b/027call_ingredient.cc
index 9e98cdc2..1bc3a563 100644
--- a/027call_ingredient.cc
+++ b/027call_ingredient.cc
@@ -33,6 +33,7 @@ for (int i = 0; i < SIZE(ingredients); ++i) {
   reagent/*copy*/ ingredient = call_instruction.ingredients.at(i);
   // End Compute Call Ingredient
   current_call().ingredients.push_back(ingredient);
+  // End Populate Call Ingredient
 }
 
 :(before "End Primitive Recipe Declarations")
diff --git a/036refcount.cc b/036refcount.cc
index c1a12525..f96213d4 100644
--- a/036refcount.cc
+++ b/036refcount.cc
@@ -17,11 +17,16 @@ def main [
 +run: {2: ("address" "number")} <- copy {0: "literal"}
 +mem: decrementing refcount of 1000: 1 -> 0
 
+:(before "End write_memory(x) Special-cases")
+update_any_refcounts(x, data);
+
 :(before "End Globals")
 //: escape hatch for a later layer
 bool Update_refcounts_in_write_memory = true;
-:(before "End write_memory(x) Special-cases")
-if (Update_refcounts_in_write_memory) {
+
+:(code)
+void update_any_refcounts(const reagent& x, const vector<double>& data) {
+  if (!Update_refcounts_in_write_memory) return;
   if (is_mu_address(x)) {
     assert(scalar(data));
     assert(x.value);
@@ -31,7 +36,6 @@ if (Update_refcounts_in_write_memory) {
   // End Update Refcounts in write_memory(x)
 }
 
-:(code)
 void update_refcounts(const reagent& old, int new_address) {
   assert(is_mu_address(old));
   update_refcounts(get_or_insert(Memory, old.value), new_address, old.type->right, payload_size(old));
@@ -101,7 +105,9 @@ def foo [
 ]
 +run: {1: ("address" "number")} <- new {number: "type"}
 +mem: incrementing refcount of 1000: 0 -> 1
-+run: {2: ("address" "number")} <- next-ingredient
++run: foo {1: ("address" "number")}
+# leave ambiguous precisely when the next increment happens; a later layer
+# will mess with that
 +mem: incrementing refcount of 1000: 1 -> 2
 +run: {1: ("address" "number")} <- new {number: "type"}
 +mem: decrementing refcount of 1000: 2 -> 1
diff --git a/037abandon.cc b/037abandon.cc
index d3fc84b8..16804bed 100644
--- a/037abandon.cc
+++ b/037abandon.cc
@@ -133,7 +133,9 @@ def foo [
 ]
 +run: {1: ("address" "number")} <- new {number: "type"}
 +mem: incrementing refcount of 1000: 0 -> 1
-+run: {2: ("address" "number")} <- next-ingredient
++run: foo {1: ("address" "number")}
+# leave ambiguous precisely when the next increment happens; a later layer
+# will mess with that
 +mem: incrementing refcount of 1000: 1 -> 2
 +run: {2: ("address" "number")} <- copy {0: "literal"}
 +mem: decrementing refcount of 1000: 2 -> 1
diff --git a/043space.cc b/043space.cc
index 9ee280d8..3a37dc1d 100644
--- a/043space.cc
+++ b/043space.cc
@@ -282,12 +282,12 @@ Update_refcounts_in_write_memory = true;
 :(code)
 bool should_update_refcounts_in_write_memory() {
   const instruction& inst = current_instruction();
+  // End should_update_refcounts_in_write_memory Special-cases For Primitives
   if (inst.operation < MAX_PRIMITIVE_RECIPES) return true;
   if (!contains_key(Recipe, inst.operation)) return true;
   const recipe& caller = get(Recipe, inst.operation);
   if (caller.steps.empty()) return true;
-  // if the recipe doesn't begin with 'local-scope', always update refcounts
-  return caller.steps.at(0).old_name != "local-scope";
+  return caller.steps.at(0).old_name != "local-scope";  // callees that call local-scope are already dealt with before return
 }
 
 bool caller_uses_product(int product_index) {
diff --git a/072scheduler.cc b/072scheduler.cc
index 61d759ea..068e1617 100644
--- a/072scheduler.cc
+++ b/072scheduler.cc
@@ -171,6 +171,7 @@ case START_RUNNING: {
     reagent/*copy*/ ingredient = current_instruction().ingredients.at(i);
     canonize_type(ingredient);
     new_routine->calls.front().ingredients.push_back(ingredient);
+    // End Populate start-running Ingredient
   }
   Routines.push_back(new_routine);
   products.resize(1);
@@ -225,6 +226,64 @@ def f2 [
 ]
 +mem: storing 4 in location 2
 
+//: more complex: refcounting management when starting up new routines
+
+:(scenario start_running_immediately_updates_refcounts_of_ingredients)
+def main [
+  local-scope
+  create-new-routine
+  switch  # make sure we run new routine before returning
+]
+def create-new-routine [
+  local-scope
+  n:address:number <- new number:type
+  *n <- copy 34
+  start-running new-routine, n
+  # refcount of n decremented
+]
+def new-routine n:address:number [
+  local-scope
+  load-ingredients
+  1:number/raw <- copy *n
+]
+# check that n wasn't reclaimed when create-new-routine returned
++mem: storing 34 in location 1
+
+//: to support the previous scenario we'll increment refcounts for all call
+//: ingredients right at call time, and stop incrementing refcounts inside
+//: next-ingredient
+:(before "End Populate Call Ingredient")
+increment_any_refcounts(ingredient, ingredients.at(i));
+:(before "End Populate start-running Ingredient")
+increment_any_refcounts(ingredient, ingredients.at(i));
+:(before "End should_update_refcounts_in_write_memory Special-cases For Primitives")
+if (inst.operation == NEXT_INGREDIENT || inst.operation == NEXT_INGREDIENT_WITHOUT_TYPECHECKING)
+  return false;
+:(code)
+void increment_any_refcounts(const reagent& x, const vector<double>& data) {
+  if (is_mu_address(x)) {
+    assert(scalar(data));
+    assert(x.value);
+    assert(!x.metadata.size);
+    increment_refcount(data.at(0));
+  }
+  if (is_mu_container(x) || is_mu_exclusive_container(x)) {
+    const container_metadata& metadata = get(Container_metadata, x.type);
+    for (map<set<tag_condition_info>, set<address_element_info> >::const_iterator p = metadata.address.begin(); p != metadata.address.end(); ++p) {
+      if (!all_match(data, p->first)) continue;
+      for (set<address_element_info>::const_iterator info = p->second.begin(); info != p->second.end(); ++info)
+        increment_refcount(data.at(info->offset));
+    }
+  }
+}
+
+void increment_refcount(int address) {
+  if (address == 0) return;
+  int refcount = get_or_insert(Memory, address);
+  trace(9999, "mem") << "incrementing refcount of " << address << ": " << refcount << " -> " << refcount+1 << end();
+  put(Memory, address, refcount+1);
+}
+
 :(scenario start_running_returns_routine_id)
 def f1 [
   1:number <- start-running f2