This makes my head hurt

docs/REFCOUNTING.md

@@ -22,6 +22,10 @@ Invariant: all paths through a function should leave each variable with 0 net re
 Special cases:
 - Most exception-related functions steal refs.  If you throw an exception in C code (via `throw [ExcInfo] e`), it consumes the refs.
 
+Some things need to be ok with their arguments going away.
+- Function objects can generally get deallocated while they are running.
+- tp_descr_get functions need to keep their first argument alive.
+
 ## Refcounting in the rewriter, baseline jit, and llvm jit
 
 ## Debugging

src/runtime/objmodel.cpp

@@ -2174,10 +2174,7 @@ Box* getattrInternalGeneric(Box* obj, BoxedString* attr, GetattrRewriteArgs* rew
         descr = typeLookup(obj->cls, attr);
     }
 
-    assert(0 && "figure out what to do about `descr`, especially in the rewrite version (and maybe _get_ too)");
-    // Keep it alive just to be safe:
-    //Py_INCREF(descr);
-    //AUTO_DECREF(descr);
+    XKEEP_ALIVE(descr);
 
     // Check if it's a data descriptor
     descrgetfunc descr_get = NULL;
@@ -2274,6 +2271,8 @@ Box* getattrInternalGeneric(Box* obj, BoxedString* attr, GetattrRewriteArgs* rew
         }
     }
 
+    XKEEP_ALIVE(_get_); // Maybe not necessary?
+
     if (!cls_only) {
         if (!IsType) {
             // Look up the val in the object's dictionary and if you find it, return it.
@@ -2709,7 +2708,7 @@ void setattrGeneric(Box* obj, BoxedString* attr, STOLEN(Box*) val, SetattrRewrit
         descr = typeLookup(obj->cls, attr);
     }
 
-    assert(0 && "figure out what to do about `descr`, especially in the rewrite version (and maybe _set_ too)");
+    XKEEP_ALIVE(descr);
 
     Box* _set_ = NULL;
     RewriterVar* r_set = NULL;
@@ -4491,6 +4490,8 @@ Box* callCLFunc(FunctionMetadata* md, CallRewriteArgs* rewrite_args, int num_out
         rewrite_args = NULL;
     }
 
+    assert(0 && "I think this is where the KEEP_ALIVE should go.  should also keep the rewritten version alive.");
+
     CompiledFunction* chosen_cf = pickVersion(md, S, num_output_args, oarg1, oarg2, oarg3, oargs);
 
     if (!chosen_cf) {
@@ -5661,11 +5662,11 @@ static Box* callItemOrSliceAttr(Box* target, BoxedString* item_str, BoxedString*
     }
 
     // Guard on the type of the object (need to have the slice operator attribute to call it).
-    Box* slice_attr = NULL;
+    bool has_slice_attr = false;
     if (rewrite_args) {
         RewriterVar* target_cls = rewrite_args->obj->getAttr(offsetof(Box, cls));
         GetattrRewriteArgs grewrite_args(rewrite_args->rewriter, target_cls, Location::any());
-        slice_attr = typeLookup(target->cls, slice_str, &grewrite_args);
+        has_slice_attr = (bool)typeLookup(target->cls, slice_str, &grewrite_args);
         if (!grewrite_args.isSuccessful()) {
             rewrite_args = NULL;
         } else {
@@ -5676,18 +5677,16 @@ static Box* callItemOrSliceAttr(Box* target, BoxedString* item_str, BoxedString*
                 rewrite_args = NULL;
 
             if (rewrite_args)
-                assert((bool)slice_attr == (return_convention == ReturnConvention::HAS_RETURN));
+                assert(has_slice_attr == (return_convention == ReturnConvention::HAS_RETURN));
         }
     } else {
-        slice_attr = typeLookup(target->cls, slice_str);
+        has_slice_attr = (bool)typeLookup(target->cls, slice_str);
     }
 
-    if (!slice_attr) {
+    if (!has_slice_attr) {
         return callItemAttr<S, rewritable>(target, item_str, slice, value, rewrite_args);
     }
 
-    assert(0 && "how to keep slice_attr alive? (esp for rewrites)");
-
     // Need a slice object to use the slice operators.
     if (rewrite_args) {
         rewrite_args->arg1->addAttrGuard(offsetof(Box, cls), (uint64_t)slice->cls);
@@ -6105,8 +6104,6 @@ extern "C" void delattrGeneric(Box* obj, BoxedString* attr, DelattrRewriteArgs*
         static BoxedString* delete_str = getStaticString("__delete__");
         Box* delAttr = typeLookup(static_cast<BoxedClass*>(clsAttr->cls), delete_str);
 
-        assert(0 && "how to keep delAttr alive (esp in rewrite)");
-
         if (delAttr != NULL) {
             Box* rtn = runtimeCallInternal<CXX, NOT_REWRITABLE>(delAttr, NULL, ArgPassSpec(2), clsAttr, obj, NULL, NULL,
                                                                 NULL);
@@ -6115,8 +6112,6 @@ extern "C" void delattrGeneric(Box* obj, BoxedString* attr, DelattrRewriteArgs*
         }
     }
 
-    assert(0 && "how to keep clsAttr alive (esp in rewrite)");
-
     // check if the attribute is in the instance's __dict__
     Box* attrVal = obj->getattr(attr);
     if (attrVal != NULL) {
@@ -6843,8 +6838,7 @@ extern "C" Box* importStar(Box* _from_module, Box* to_globals) {
         if (!all_getitem)
             raiseExcHelper(TypeError, "'%s' object does not support indexing", getTypeName(all));
 
-        Py_INCREF(all_getitem);
-        AUTO_DECREF(all_getitem);
+        KEEP_ALIVE(all_getitem);
 
         int idx = 0;
         while (true) {

src/runtime/types.cpp

@@ -817,8 +817,8 @@ static Box* typeCallInner(CallRewriteArgs* rewrite_args, ArgPassSpec argspec, Bo
     RewriterVar* r_ccls = NULL;
     RewriterVar* r_new = NULL;
     RewriterVar* r_init = NULL;
-    // TODO: is this ok?  what if new causes init to get freed?
-    BORROWED(Box*) init_attr = NULL;
+    // TODO: shouldn't cache it like this.  What if __new__ changes __init__
+    DecrefHandle<Box, true> init_attr(nullptr);
     if (rewrite_args) {
         assert(!argspec.has_starargs);
         assert(!argspec.has_kwargs);
@@ -997,7 +997,7 @@ static Box* typeCallInner(CallRewriteArgs* rewrite_args, ArgPassSpec argspec, Bo
         // tp_init if we can manage to rewrite it.
         if (rewrite_args && which_init != UNKNOWN) {
             GetattrRewriteArgs grewrite_args(rewrite_args->rewriter, r_ccls, rewrite_args->destination);
-            init_attr = typeLookup(cls, init_str, &grewrite_args);
+            init_attr = incref(typeLookup(cls, init_str, &grewrite_args));
 
             if (!grewrite_args.isSuccessful())
                 rewrite_args = NULL;
@@ -1010,7 +1010,7 @@ static Box* typeCallInner(CallRewriteArgs* rewrite_args, ArgPassSpec argspec, Bo
                 }
             }
         } else {
-            init_attr = typeLookup(cls, init_str);
+            init_attr = incref(typeLookup(cls, init_str));
         }
     }
 

src/runtime/types.h

@@ -380,6 +380,7 @@ template <typename B> DecrefHandle<B, true> autoXDecref(B* b) {
 #define AUTO_XDECREF(x) DecrefHandle<Box, true> CAT(_autodecref_, __LINE__)((x))
 
 #define KEEP_ALIVE(x) AUTO_DECREF(incref(x))
+#define XKEEP_ALIVE(x) AUTO_XDECREF(xincref(x))
 
 template <bool Nullable = false> class AutoDecrefArray {
 private:

test/tests/descr_selfdelete.py

@@ -13,14 +13,18 @@
 #   So also test it by creating a closure and make sure that that stays alive.
 # - This file wraps each individual test in a function and then calls it twice -- this is to try to test
 #   any rewritten (traced) variants as well.
+#
+# Actually, looking more into CPython's behavior, it seems like CPython is very tolerant of executing a
+# function object that gets deallocated during its own execution: CPython copies all the relevant info
+# into the frame and then accesses it from there.
 
 import sys
 
 def make_closure(f):
     def inner(*args, **kw):
-        print f.__name__
+        print "starting", f.__name__
         r = f(*args, **kw)
-        print f.__name__
+        print "ending", f.__name__
         return r
     return inner
 
@@ -159,8 +163,43 @@ f()
 f()
 f()
 
+def f():
+    class D(object):
+        @make_closure
+        def __get__(self, *args):
+            print "D.__get__"
+            del C.__contains__
+            del D.__get__
+            print self
+            return lambda *args: 1
+
+    class C(object):
+        __contains__ = make_closure(D())
+
+    print 1 in C()
+f()
+f()
+f()
 
 
+def f():
+    class D(object):
+        @make_closure
+        def __get__(self, *args):
+            print "D.__getattribute__"
+            del C.__getattribute__
+            del D.__get__
+            print self
+            return lambda *args: 1
+
+    class C(object):
+        __getattribute__ = D()
+
+    print C().a
+f()
+f()
+f()
+
 # Related:
 def f():
     @make_closure