Switch to CPython's implementation of thread._local

CPython's implementation has quite a bit more features than our
old one.  We only particularly need one of them (call __init__
when accessed from a new thread), but it looks like there are
some other features in there that have a decent chance of biting
us in annoying ways (some gc-related stuff).

That implementation forced some of the other work in this PR, of
supporting weakrefs on extension objects (which this uses), and
making object.tp_init get set the same way it does in CPython.

from_cpython/CMakeLists.txt

@@ -64,6 +64,7 @@ file(GLOB_RECURSE STDMODULE_SRCS Modules
     stringio.c
     stropmodule.c
     textio.c
+    threadmodule.c
     timemodule.c
     unicodedata.c
     util.c

from_cpython/Modules/threadmodule.c

@@ -13,8 +13,10 @@
 
 #include "pythread.h"
 
-static PyObject *ThreadError;
+// Pyston change: we're only using part of this file
 static PyObject *str_dict;
+#if 0
+static PyObject *ThreadError;
 static long nb_threads = 0;
 
 /* Lock objects */
@@ -173,6 +175,7 @@ newlockobject(void)
     }
     return self;
 }
+#endif
 
 /* Thread-local objects */
 
@@ -236,7 +239,7 @@ static PyTypeObject localdummytype = {
     /* tp_name           */ "_thread._localdummy",
     /* tp_basicsize      */ sizeof(localdummyobject),
     /* tp_itemsize       */ 0,
-    /* tp_dealloc        */ (destructor)localdummy_dealloc,
+    /* tp_dealloc        */ (destructor)/*localdummy_dealloc*/ NULL,
     /* tp_print          */ 0,
     /* tp_getattr        */ 0,
     /* tp_setattr        */ 0,
@@ -389,6 +392,10 @@ local_traverse(localobject *self, visitproc visit, void *arg)
 static int
 local_clear(localobject *self)
 {
+    // Pyston change:
+    Py_FatalError("unexpected call to local_clear()");
+    abort();
+#if 0
     PyThreadState *tstate;
     Py_CLEAR(self->args);
     Py_CLEAR(self->kw);
@@ -406,6 +413,7 @@ local_clear(localobject *self)
                 PyDict_DelItem(tstate->dict, self->key);
     }
     return 0;
+#endif
 }
 
 static void
@@ -490,7 +498,7 @@ static PyTypeObject localtype = {
     /* tp_name           */ "thread._local",
     /* tp_basicsize      */ sizeof(localobject),
     /* tp_itemsize       */ 0,
-    /* tp_dealloc        */ (destructor)local_dealloc,
+    /* tp_dealloc        */ (destructor)/*local_dealloc*/ NULL,
     /* tp_print          */ 0,
     /* tp_getattr        */ 0,
     /* tp_setattr        */ 0,
@@ -589,6 +597,8 @@ _localdummy_destroyed(PyObject *localweakref, PyObject *dummyweakref)
     Py_RETURN_NONE;
 }
 
+// Pyston change:
+#if 0
 /* Module functions */
 
 struct bootstate {
@@ -846,8 +856,11 @@ requiring allocation in multiples of the system memory page size\n\
 - platform documentation should be referred to for more information\n\
 (4kB pages are common; using multiples of 4096 for the stack size is\n\
 the suggested approach in the absence of more specific information).");
+#endif
 
 static PyMethodDef thread_methods[] = {
+// Pyston change:
+#if 0
     {"start_new_thread",        (PyCFunction)thread_PyThread_start_new_thread,
                             METH_VARARGS,
                             start_new_doc},
@@ -871,6 +884,7 @@ static PyMethodDef thread_methods[] = {
     {"stack_size",              (PyCFunction)thread_stack_size,
                             METH_VARARGS,
                             stack_size_doc},
+#endif
     {NULL,                      NULL}           /* sentinel */
 };
 
@@ -909,6 +923,8 @@ initthread(void)
     if (m == NULL)
         return;
 
+    // Pyston change:
+#if 0
     /* Add a symbolic constant */
     d = PyModule_GetDict(m);
     ThreadError = PyGC_AddRoot(PyErr_NewException("thread.error", NULL, NULL));
@@ -918,17 +934,18 @@ initthread(void)
         return;
     Py_INCREF(&Locktype);
     PyDict_SetItemString(d, "LockType", (PyObject *)&Locktype);
+#endif
 
     Py_INCREF(&localtype);
     if (PyModule_AddObject(m, "_local", (PyObject *)&localtype) < 0)
         return;
 
-    nb_threads = 0;
+    //nb_threads = 0; // pyston change
 
     str_dict = PyGC_AddRoot(PyString_InternFromString("__dict__"));
     if (str_dict == NULL)
         return;
 
     /* Initialize the C thread library */
-    PyThread_init_thread();
+    //PyThread_init_thread(); // pyston change
 }

src/runtime/builtin_modules/thread.cpp

@@ -27,6 +27,8 @@
 
 using namespace pyston::threading;
 
+extern "C" void initthread();
+
 
 static int initialized;
 static void PyThread__init_thread(void); /* Forward */
@@ -57,8 +59,6 @@ static size_t _pythread_stacksize = 0;
 
 namespace pyston {
 
-BoxedModule* thread_module;
-
 static void* thread_start(Box* target, Box* varargs, Box* kwargs) {
     assert(target);
     assert(varargs);
@@ -156,73 +156,6 @@ Box* allocateLock() {
     return new BoxedThreadLock();
 }
 
-static BoxedClass* thread_local_cls;
-class BoxedThreadLocal : public Box {
-public:
-    BoxedThreadLocal() {}
-
-    static Box* getThreadLocalObject(Box* obj) {
-        BoxedDict* dict = static_cast<BoxedDict*>(PyThreadState_GetDict());
-        Box* tls_obj = dict->getOrNull(obj);
-        if (tls_obj == NULL) {
-            tls_obj = new BoxedDict();
-            setitem(dict, obj, tls_obj);
-        }
-        return tls_obj;
-    }
-
-    static Box* setattrPyston(Box* obj, Box* name, Box* val) noexcept {
-        if (isSubclass(name->cls, str_cls) && static_cast<BoxedString*>(name)->s() == "__dict__") {
-            raiseExcHelper(AttributeError, "'%.50s' object attribute '__dict__' is read-only", Py_TYPE(obj)->tp_name);
-        }
-
-        Box* tls_obj = getThreadLocalObject(obj);
-        setitem(tls_obj, name, val);
-        return None;
-    }
-
-    static int setattro(Box* obj, Box* name, Box* val) noexcept {
-        try {
-            setattrPyston(obj, name, val);
-        } catch (ExcInfo e) {
-            setCAPIException(e);
-            return -1;
-        }
-        return 0;
-    }
-
-    static Box* getattro(Box* obj, Box* name) noexcept {
-        assert(name->cls == str_cls);
-        llvm::StringRef s = static_cast<BoxedString*>(name)->s();
-
-        Box* tls_obj = getThreadLocalObject(obj);
-        if (s == "__dict__")
-            return tls_obj;
-
-        try {
-            return getitem(tls_obj, name);
-        } catch (ExcInfo e) {
-        }
-
-        try {
-            Box* r = getattrInternalGeneric(obj, s, NULL, false, false, NULL, NULL);
-            if (r)
-                return r;
-        } catch (ExcInfo e) {
-            setCAPIException(e);
-            return NULL;
-        }
-
-        PyErr_Format(PyExc_AttributeError, "'%.50s' object has no attribute '%.400s'", Py_TYPE(obj)->tp_name, s.data());
-        return NULL;
-    }
-
-    static Box* hash(Box* obj) { return boxInt(PyThread_get_thread_ident()); }
-
-
-    DEFAULT_CLASS(thread_local_cls);
-};
-
 Box* getIdent() {
     return boxInt(pthread_self());
 }
@@ -232,7 +165,14 @@ Box* stackSize() {
 }
 
 void setupThread() {
-    thread_module = createModule("thread");
+    // Hacky: we want to use some of CPython's implementation of the thread module (the threading local stuff),
+    // and some of ours (thread handling).  Start off by calling a cut-down version of initthread, and then
+    // add our own attributes to the module it creates.
+    initthread();
+    RELEASE_ASSERT(!PyErr_Occurred(), "");
+
+    Box* thread_module = getSysModulesDict()->getOrNull(boxString("thread"));
+    assert(thread_module);
 
     thread_module->giveAttr("start_new_thread", new BoxedBuiltinFunctionOrMethod(
                                                     boxRTFunction((void*)startNewThread, BOXED_INT, 3, 1, false, false),
@@ -256,21 +196,6 @@ void setupThread() {
     thread_lock_cls->giveAttr("__exit__", new BoxedFunction(boxRTFunction((void*)BoxedThreadLock::exit, NONE, 4)));
     thread_lock_cls->freeze();
 
-    thread_local_cls = new (0) BoxedHeapClass(object_cls, NULL, 0, 0, sizeof(BoxedThreadLocal), false,
-                                              static_cast<BoxedString*>(boxString("_local")));
-    thread_local_cls->giveAttr("__module__", boxStrConstant("thread"));
-    thread_local_cls->giveAttr("__hash__",
-                               new BoxedFunction(boxRTFunction((void*)BoxedThreadLocal::hash, BOXED_INT, 1)));
-    thread_local_cls->giveAttr("__setattr__",
-                               new BoxedFunction(boxRTFunction((void*)BoxedThreadLocal::setattrPyston, UNKNOWN, 3)));
-    thread_module->giveAttr("_local", thread_local_cls);
-
-    thread_local_cls->tp_setattro = BoxedThreadLocal::setattro;
-    thread_local_cls->tp_getattro = BoxedThreadLocal::getattro;
-    add_operators(thread_local_cls);
-    thread_local_cls->finishInitialization();
-    thread_local_cls->freeze();
-
     BoxedClass* ThreadError
         = BoxedHeapClass::create(type_cls, Exception, NULL, Exception->attrs_offset, Exception->tp_weaklistoffset,
                                  Exception->tp_basicsize, false, "error");

src/runtime/capi.cpp

@@ -258,15 +258,215 @@ extern "C" PyObject* PyObject_GenericGetAttr(PyObject* o, PyObject* name) noexce
     }
 }
 
+// Note (kmod): I don't feel great about including an alternate code-path for lookups.  I also, however, don't feel
+// great about modifying our code paths to take a custom dict, and since this code is just copied from CPython
+// I feel like the risk is pretty low.
 extern "C" PyObject* _PyObject_GenericGetAttrWithDict(PyObject* obj, PyObject* name, PyObject* dict) noexcept {
-    fatalOrError(PyExc_NotImplementedError, "unimplemented");
-    return nullptr;
+    PyTypeObject* tp = Py_TYPE(obj);
+    PyObject* descr = NULL;
+    PyObject* res = NULL;
+    descrgetfunc f;
+    Py_ssize_t dictoffset;
+    PyObject** dictptr;
+
+    if (!PyString_Check(name)) {
+#ifdef Py_USING_UNICODE
+        /* The Unicode to string conversion is done here because the
+           existing tp_setattro slots expect a string object as name
+           and we wouldn't want to break those. */
+        if (PyUnicode_Check(name)) {
+            name = PyUnicode_AsEncodedString(name, NULL, NULL);
+            if (name == NULL)
+                return NULL;
+        } else
+#endif
+        {
+            PyErr_Format(PyExc_TypeError, "attribute name must be string, not '%.200s'", Py_TYPE(name)->tp_name);
+            return NULL;
+        }
+    } else
+        Py_INCREF(name);
+
+    if (tp->tp_dict == NULL) {
+        if (PyType_Ready(tp) < 0)
+            goto done;
+    }
+
+#if 0 /* XXX this is not quite _PyType_Lookup anymore */
+    /* Inline _PyType_Lookup */
+    {
+        Py_ssize_t i, n;
+        PyObject *mro, *base, *dict;
+
+        /* Look in tp_dict of types in MRO */
+        mro = tp->tp_mro;
+        assert(mro != NULL);
+        assert(PyTuple_Check(mro));
+        n = PyTuple_GET_SIZE(mro);
+        for (i = 0; i < n; i++) {
+            base = PyTuple_GET_ITEM(mro, i);
+            if (PyClass_Check(base))
+                dict = ((PyClassObject *)base)->cl_dict;
+            else {
+                assert(PyType_Check(base));
+                dict = ((PyTypeObject *)base)->tp_dict;
+            }
+            assert(dict && PyDict_Check(dict));
+            descr = PyDict_GetItem(dict, name);
+            if (descr != NULL)
+                break;
+        }
+    }
+#else
+    descr = _PyType_Lookup(tp, name);
+#endif
+
+    Py_XINCREF(descr);
+
+    f = NULL;
+    if (descr != NULL && PyType_HasFeature(descr->cls, Py_TPFLAGS_HAVE_CLASS)) {
+        f = descr->cls->tp_descr_get;
+        if (f != NULL && PyDescr_IsData(descr)) {
+            res = f(descr, obj, (PyObject*)obj->cls);
+            Py_DECREF(descr);
+            goto done;
+        }
+    }
+
+    if (dict == NULL) {
+        /* Inline _PyObject_GetDictPtr */
+        dictoffset = tp->tp_dictoffset;
+        if (dictoffset != 0) {
+            if (dictoffset < 0) {
+                Py_ssize_t tsize;
+                size_t size;
+
+                tsize = ((PyVarObject*)obj)->ob_size;
+                if (tsize < 0)
+                    tsize = -tsize;
+                size = _PyObject_VAR_SIZE(tp, tsize);
+
+                dictoffset += (long)size;
+                assert(dictoffset > 0);
+                assert(dictoffset % SIZEOF_VOID_P == 0);
+            }
+            dictptr = (PyObject**)((char*)obj + dictoffset);
+            dict = *dictptr;
+        }
+    }
+    if (dict != NULL) {
+        Py_INCREF(dict);
+        res = PyDict_GetItem(dict, name);
+        if (res != NULL) {
+            Py_INCREF(res);
+            Py_XDECREF(descr);
+            Py_DECREF(dict);
+            goto done;
+        }
+        Py_DECREF(dict);
+    }
+
+    if (f != NULL) {
+        res = f(descr, obj, (PyObject*)Py_TYPE(obj));
+        Py_DECREF(descr);
+        goto done;
+    }
+
+    if (descr != NULL) {
+        res = descr;
+        /* descr was already increfed above */
+        goto done;
+    }
+
+    PyErr_Format(PyExc_AttributeError, "'%.50s' object has no attribute '%.400s'", tp->tp_name,
+                 PyString_AS_STRING(name));
+done:
+    Py_DECREF(name);
+    return res;
 }
 
+// (see note for _PyObject_GenericGetAttrWithDict)
 extern "C" int _PyObject_GenericSetAttrWithDict(PyObject* obj, PyObject* name, PyObject* value,
                                                 PyObject* dict) noexcept {
-    fatalOrError(PyExc_NotImplementedError, "unimplemented");
-    return -1;
+    PyTypeObject* tp = Py_TYPE(obj);
+    PyObject* descr;
+    descrsetfunc f;
+    PyObject** dictptr;
+    int res = -1;
+
+    if (!PyString_Check(name)) {
+#ifdef Py_USING_UNICODE
+        /* The Unicode to string conversion is done here because the
+           existing tp_setattro slots expect a string object as name
+           and we wouldn't want to break those. */
+        if (PyUnicode_Check(name)) {
+            name = PyUnicode_AsEncodedString(name, NULL, NULL);
+            if (name == NULL)
+                return -1;
+        } else
+#endif
+        {
+            PyErr_Format(PyExc_TypeError, "attribute name must be string, not '%.200s'", Py_TYPE(name)->tp_name);
+            return -1;
+        }
+    } else
+        Py_INCREF(name);
+
+    if (tp->tp_dict == NULL) {
+        if (PyType_Ready(tp) < 0)
+            goto done;
+    }
+
+    descr = _PyType_Lookup(tp, name);
+    f = NULL;
+    if (descr != NULL && PyType_HasFeature(descr->cls, Py_TPFLAGS_HAVE_CLASS)) {
+        f = descr->cls->tp_descr_set;
+        if (f != NULL && PyDescr_IsData(descr)) {
+            res = f(descr, obj, value);
+            goto done;
+        }
+    }
+
+    if (dict == NULL) {
+        dictptr = _PyObject_GetDictPtr(obj);
+        if (dictptr != NULL) {
+            dict = *dictptr;
+            if (dict == NULL && value != NULL) {
+                dict = PyDict_New();
+                if (dict == NULL)
+                    goto done;
+                *dictptr = dict;
+            }
+        }
+    }
+    if (dict != NULL) {
+        Py_INCREF(dict);
+        if (value == NULL)
+            res = PyDict_DelItem(dict, name);
+        else
+            res = PyDict_SetItem(dict, name, value);
+        if (res < 0 && PyErr_ExceptionMatches(PyExc_KeyError))
+            PyErr_SetObject(PyExc_AttributeError, name);
+        Py_DECREF(dict);
+        goto done;
+    }
+
+    if (f != NULL) {
+        res = f(descr, obj, value);
+        goto done;
+    }
+
+    if (descr == NULL) {
+        PyErr_Format(PyExc_AttributeError, "'%.100s' object has no attribute '%.200s'", tp->tp_name,
+                     PyString_AS_STRING(name));
+        goto done;
+    }
+
+    PyErr_Format(PyExc_AttributeError, "'%.50s' object attribute '%.400s' is read-only", tp->tp_name,
+                 PyString_AS_STRING(name));
+done:
+    Py_DECREF(name);
+    return res;
 }
 
 

src/runtime/objmodel.cpp

@@ -901,6 +901,16 @@ void Box::setattr(const std::string& attr, Box* val, SetattrRewriteArgs* rewrite
     abort();
 }
 
+extern "C" PyObject* _PyType_Lookup(PyTypeObject* type, PyObject* name) noexcept {
+    RELEASE_ASSERT(name->cls == str_cls, "");
+    try {
+        return typeLookup(type, static_cast<BoxedString*>(name)->s(), NULL);
+    } catch (ExcInfo e) {
+        setCAPIException(e);
+        return NULL;
+    }
+}
+
 Box* typeLookup(BoxedClass* cls, const std::string& attr, GetattrRewriteArgs* rewrite_args) {
     Box* val;
 

test/tests/threading_local.py

@@ -12,12 +12,14 @@ class CustomThreadingLocal(threading.local):
         self.n += 1
         print self.a, self.n
 print CustomThreadingLocal().a
-print CustomThreadingLocal().a
+c = CustomThreadingLocal()
+print c.a
 
 def f():
+    print
     a.x = "goodbye world"
     print a.x
-    print CustomThreadingLocal().a
+    print c.a
     print CustomThreadingLocal().a
 
 def test():
@@ -36,3 +38,6 @@ print a.x
 a.__setattr__('x', 5)
 print a.x
 print sorted(a.__dict__.items())
+
+del a.x
+print sorted(a.__dict__.items())