Fix bug794133

This commit fixes bug794133, though in a suboptimal way.

Bug 794133 noted that when one declares an array of arrays as
follows:

        var A: [D] [1..2] real;

that the reference count for the domain {1..2} is incremented
once per index in D.  This means that if D is empty, the
anonymous domain {1..2} will have a reference count of zero
and be deallocated when it leaves scope, even if A persists
longer (through an alias, returning it from a procedure, etc.)
This is problematic because the domain will need to persist
as long as A does since D could be resized over time (and in
fact, was in the bug).

This commit fixes the issue by adding an initialize() routine
for arrays and logic in their destructors to check whether the
element type is an array and bump the reference count of its
domain up and down if it is.  It also calls itself recursively
on the element type of that array to handle the case where it's
an array of arrays of arrays ...

This fixes not only the original bug, but a number of variations
on it that I introduced while studying it.

The reason the fix is suboptimal is that, given a 'type t'
that is array type, one would really like to query the domain
and element type from it without creating an array -- and
one should be able to do this through primitives or, possibly,
externs, it's just tricky.  While wrestling with this trickiness,
I wanted to check in a fix for the upcoming release (in case
I don't finish) which takes the simpler approach of declaring
a temporary variable of type 't' and then queries the domain
and element type from that.  This has the downside of the
overhead of allocating and freeing the variable, though it
should only be incurred in the event that we're dealing with
arrays of arrays.

The challenge of making a primitive to do this query is that
you have to dig into the _array record type within the
compiler via its generic fields in order to get to the
domain and element type; I couldn't get this working
quickly enough.  So then I switched to writing an extern
function in C that would get the domain from the runtime
array type, and got this working in the single-locale
case, but it's more complicated in the multi-locale case
due to wide pointers and privatization.  Having run into
those issues (and the fact that the eltType can't be
queried at runtime for a basic array type), I think the
primitive is the way to go, but haven't gotten back to it
yet.

One open question that this fix raises is whether we still
need to bump the reference count of {1..2} once per index
in D or not.  I'm not certain of the answer.  For the array
A itself, it's clear that it need not be; but the question
would be whether, if one of the elements might outlive A,
for example through a slice or alias or reference, it would
require that reference count to be bumped at A's creation
time or whether it would be bumped when that reference was
taken (or, ideally, when that reference was taken and
might escape through asynchronous parallelism).



git-svn-id: http://svn.code.sf.net/p/chapel/code/trunk@21192 3a8e244f-b0f2-452b-bcba-4c88e055c3ca

modules/internal/ChapelArray.chpl

@@ -282,6 +282,46 @@ module ChapelArray {
       help();
     return D;
   }
+
+
+  //
+  // These routines increment and decrement the reference count
+  // for a domain that is part of an array's element type.
+  // Prior to introducing these routines and calls, we would
+  // increment/decrement the reference count based on the
+  // number of indices in the outer domain instead; this could
+  // cause the domain to be deallocated prematurely in the
+  // case the the outer domain was empty.  For example:
+  //
+  //   var D = {1..0};   // start empty; we'll resize later
+  //   var A: [D] [1..2] real;
+  //
+  // The anonymous domain {1..2} must be kept alive as a result
+  // of being part of A's type even though D is initially empty.
+  // Thus, {1..2} should remain alive as long as A is.  By
+  // incrementing and decrementing its reference counts based
+  // on A's lifetime rather than the number of elements in domain
+  // D, we ensure that is kept alive.  See
+  // test/users/bugzilla/bug794133/ for more details and examples.
+  //
+  proc chpl_incRefCountsForDomainsInArrayEltTypes(type eltType) {
+    if (isArrayType(eltType)) {
+      var ev: eltType;
+      ev.domain._value._domCnt.add(1);
+      chpl_incRefCountsForDomainsInArrayEltTypes(ev.eltType);
+    }
+  }
+
+  proc chpl_decRefCountsForDomainsInArrayEltTypes(type eltType) {
+    if (isArrayType(eltType)) {
+      var ev: eltType;
+      const refcount = ev.domain._value.destroyDom();
+      if !noRefCount then
+        if refcount == 0 then
+          delete ev.domain._value;
+      chpl_decRefCountsForDomainsInArrayEltTypes(ev.eltType);
+    }
+  }
 
   //
   // Support for subdomain types
@@ -1138,22 +1178,35 @@ module ChapelArray {
     var _value;     // stores array class, may be privatized
     var _valueType; // stores type of privatized arrays
     var _promotionType: _value.eltType;
-
+
+    proc initialize() {
+      chpl_incRefCountsForDomainsInArrayEltTypes(_value.eltType);
+    }
+
     inline proc _value {
       if _isPrivatized(_valueType) {
         return chpl_getPrivatizedCopy(_valueType.type, _value);
       } else {
         return _value;
       }
     }
-
+
+    //
+    // Note that the destructor may be called multiple times for
+    // a given array, corresponding to cases in which it's
+    // autodestroyed multiple times; only the case that brings
+    // the reference count to zero is the one that should
+    // actually free the array.
+    //
     proc ~_array() {
       if !_isPrivatized(_valueType) {
         on _value {
           var cnt = _value.destroyArr();
           if !noRefCount then
-            if cnt == 0 then
+            if cnt == 0 then {
+              chpl_decRefCountsForDomainsInArrayEltTypes(_value.eltType);
               delete _value;
+            }
         }
       }
     }