3993

Fully isolate routines from their arguments.

I still need exceptions for containers that are *designed* to be
shared between routines. The primary such case is channels; we need some
way to share them between routines, and if we deep-copy them that
defeats their entire purpose.

A milder case is the use of fake file-systems in tests, though that's a
hint that there'll be more of these as the OS gets more fleshed out. The
pattern seems to be that we need to not deep-copy containers that
contain lock fields, and so their operations internally do their own
locking.

We may have to stop hard-coding the list of exceptions and allow people
to define new ones. Perhaps don't deep-copy any container with metadata
of 'shared', and then ensure that get-location is only ever called on
shared containers.

This still isn't absolutely ironclad. People can now store something
into a channel and then pass it into a routine to share arbitrary data.
But perhaps the goal isn't to be ironclad, just to avoid easy mistakes.
I'd still want an automated check for this, though. Some way to
highlight it as an unsafe pattern.

This completes step 1 in the plan of commit 3992 for making
continuations safe.

2 files changed, 84 insertions, 30 deletions

diff --git a/071deep_copy.cc b/071deep_copy.cc
index cc3a3424..d403200b 100644
--- a/071deep_copy.cc
+++ b/071deep_copy.cc
@@ -210,27 +210,33 @@ case DEEP_COPY: {
 }
 :(before "End Primitive Recipe Implementations")
 case DEEP_COPY: {
-  const reagent& input = current_instruction().ingredients.at(0);
+  products.push_back(deep_copy(current_instruction().ingredients.at(0)));
+  break;
+}
+
+:(code)
+vector<double> deep_copy(const reagent& in) {
   // allocate a tiny bit of temporary space for deep_copy()
   trace(9991, "run") << "deep-copy: allocating space for temporary" << end();
   reagent tmp("tmp:address:number");
   tmp.set_value(allocate(1));
-  products.push_back(deep_copy(input, tmp));
+  map<int, int> addresses_copied;
+  vector<double> result = deep_copy(in, addresses_copied, tmp);
   // reclaim Mu memory allocated for tmp
   trace(9991, "run") << "deep-copy: reclaiming temporary" << end();
   abandon(tmp.value, payload_type(tmp.type), payload_size(tmp));
   // reclaim host memory allocated for tmp.type when tmp goes out of scope
-  break;
-}
-
-:(code)
-vector<double> deep_copy(const reagent& in, const reagent& tmp) {
-  map<int, int> addresses_copied;
-  return deep_copy(in, addresses_copied, tmp);
+  return result;
 }
 
 vector<double> deep_copy(reagent/*copy*/ in, map<int, int>& addresses_copied, const reagent& tmp) {
   canonize(in);
+  if (is_mu_address(in)) {
+    // hack: skip primitive containers that do their own locking; they're designed to be shared between routines
+    type_tree* payload = in.type->right;
+    if (payload->left->name == "channel" || payload->left->name == "resources")
+      return read_memory(in);
+  }
   vector<double> result;
   if (is_mu_address(in))
     result.push_back(deep_copy_address(in, addresses_copied, tmp));
@@ -280,6 +286,11 @@ void deep_copy(const reagent& canonized_in, map<int, int>& addresses_copied, con
   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) {
+      // hack: skip primitive containers that do their own locking; they're designed to be shared between routines
+      if (!info->payload_type->atom && info->payload_type->left->name == "channel")
+        continue;
+      if (info->payload_type->atom && info->payload_type->name == "resources")
+        continue;
       // construct a fake reagent that reads directly from the appropriate
       // field of the container
       reagent curr;
@@ -301,8 +312,6 @@ int payload_address(reagent/*copy*/ x) {
   return x.value;
 }
 
-//: moar tests, just because I can't believe it all works
-
 :(scenario deep_copy_stress_test_1)
 container foo1 [
   p:&:num
@@ -388,3 +397,61 @@ def main [
 +mem: storing 1 in location 2
 # but it's a completely different (disjoint) cycle
 +mem: storing 0 in location 3
+
+:(scenario deep_copy_skips_channel)
+# ugly! dummy 'channel' type if we happen to be building without that layer
+% if (!contains_key(Type_ordinal, "channel")) get_or_insert(Type, put(Type_ordinal, "channel", Next_type_ordinal++)).name = "channel";
+def main [
+  local-scope
+  x:&:channel:num <- new {(channel num): type}
+  y:&:channel:num <- deep-copy x
+  10:bool/raw <- equal x, y
+]
+# channels are never deep-copied
++mem: storing 1 in location 10
+
+:(scenario deep_copy_skips_nested_channel)
+# ugly! dummy 'channel' type if we happen to be building without that layer
+% if (!contains_key(Type_ordinal, "channel")) get_or_insert(Type, put(Type_ordinal, "channel", Next_type_ordinal++)).name = "channel";
+container foo [
+  c:&:channel:num
+]
+def main [
+  local-scope
+  x:&:foo <- new foo:type
+  y:&:foo <- deep-copy x
+  xc:&:channel:num <- get *x, c:offset
+  yc:&:channel:num <- get *y, c:offset
+  10:bool/raw <- equal xc, yc
+]
+# channels are never deep-copied
++mem: storing 1 in location 10
+
+:(scenario deep_copy_skips_resources)
+# ugly! dummy 'resources' type if we happen to be building without that layer
+% if (!contains_key(Type_ordinal, "resources")) get_or_insert(Type, put(Type_ordinal, "resources", Next_type_ordinal++)).name = "resources";
+def main [
+  local-scope
+  x:&:resources <- new resources:type
+  y:&:resources <- deep-copy x
+  10:bool/raw <- equal x, y
+]
+# resources are never deep-copied
++mem: storing 1 in location 10
+
+:(scenario deep_copy_skips_nested_resources)
+# ugly! dummy 'resources' type if we happen to be building without that layer
+% if (!contains_key(Type_ordinal, "resources")) get_or_insert(Type, put(Type_ordinal, "resources", Next_type_ordinal++)).name = "resources";
+container foo [
+  c:&:resources
+]
+def main [
+  local-scope
+  x:&:foo <- new foo:type
+  y:&:foo <- deep-copy x
+  xc:&:resources <- get *x, c:offset
+  yc:&:resources <- get *y, c:offset
+  10:bool/raw <- equal xc, yc
+]
+# resources are never deep-copied
++mem: storing 1 in location 10
diff --git a/073scheduler.cc b/073scheduler.cc
index 962ac915..fb1a3f51 100644
--- a/073scheduler.cc
+++ b/073scheduler.cc
@@ -179,9 +179,10 @@ case START_RUNNING: {
   new_routine->parent_index = Current_routine_index;
   // populate ingredients
   for (int i = /*skip callee*/1;  i < SIZE(current_instruction().ingredients);  ++i) {
-    new_routine->calls.front().ingredient_atoms.push_back(ingredients.at(i));
     reagent/*copy*/ ingredient = current_instruction().ingredients.at(i);
     new_routine->calls.front().ingredients.push_back(ingredient);
+    vector<double> new_ingredient_atoms = deep_copy(ingredient);
+    new_routine->calls.front().ingredient_atoms.push_back(new_ingredient_atoms);
     // End Populate start-running Ingredient
   }
   Routines.push_back(new_routine);
@@ -252,7 +253,7 @@ def f2 n:&:num [
 :(before "End is_indirect_call_with_ingredients Special-cases")
 if (r == START_RUNNING) return true;
 
-//: more complex: refcounting management when starting up new routines
+//: refcounting management when starting up new routines
 
 :(scenario start_running_immediately_updates_refcounts_of_ingredients)
 % Scheduling_interval = 1;
@@ -275,24 +276,10 @@ def new-routine n:&:num [
   load-ingredients
   1:num/raw <- copy *n
 ]
-# check that n wasn't reclaimed when create-new-routine returned
+# check that n was successfully passed into new-routine before being reclaimed
 +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));
-:(after "should_update_refcounts() Special-cases When Writing Products Of Primitive Instructions")
-if (inst.operation == NEXT_INGREDIENT || inst.operation == NEXT_INGREDIENT_WITHOUT_TYPECHECKING) {
-  if (space_index(inst.products.at(0)) > 0) return true;
-  if (has_property(inst.products.at(0), "raw")) return true;
-  return false;
-}
-
-// ensure this works with indirect calls using 'call' as well
+//: ensure this works with indirect calls using 'call' as well
 :(scenario start_running_immediately_updates_refcounts_of_ingredients_of_indirect_calls)
 % Scheduling_interval = 1;
 def main [
@@ -306,7 +293,7 @@ def f1 n:&:num [
   local-scope
   load-ingredients
 ]
-# check that n wasn't reclaimed when f1 returned
+# check that n was successfully passed into new-routine before being reclaimed
 +mem: storing 34 in location 1
 
 :(scenario next_ingredient_never_leaks_refcounts)