bpo-1635741: Refactor _threadmodule.c (GH-23793)

* Fix ExceptHookArgsType name: "_thread.ExceptHookArgs", instead of
  "_thread._ExceptHookArgs".
* PyInit__thread() no longer intializes interp->num_threads to 0:
  it is already done in PyInterpreterState_New().
* Use PyModule_AddType(), Py_NewRef() and Py_XNewRef().
* Replace str_dict variable with _Py_IDENTIFIER(__dict__).
* Remove assert(Py_IS_TYPE(obj, &Locktype)) from release_sentinel()
  to avoid having to retrive the type from this callback.
* Add thread_bootstate_free()
* Rename t_bootstrap() to thread_run()
* bootstate structure: rename keyw member to kwargs

1 files changed, 226 insertions, 311 deletions

diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c
index 86d5f54..bd4331c 100644
--- a/Modules/_threadmodule.c
+++ b/Modules/_threadmodule.c
@@ -8,12 +8,15 @@
 #include "pycore_pystate.h"       // _PyThreadState_Init()
 #include <stddef.h>               // offsetof()
 
-static PyObject *ThreadError;
-static PyObject *str_dict;
+// ThreadError is just an alias to PyExc_RuntimeError
+#define ThreadError PyExc_RuntimeError
+
+_Py_IDENTIFIER(__dict__);
 
 _Py_IDENTIFIER(stderr);
 _Py_IDENTIFIER(flush);
 
+
 /* Lock objects */
 
 typedef struct {
@@ -26,8 +29,9 @@ typedef struct {
 static void
 lock_dealloc(lockobject *self)
 {
-    if (self->in_weakreflist != NULL)
+    if (self->in_weakreflist != NULL) {
         PyObject_ClearWeakRefs((PyObject *) self);
+    }
     if (self->lock_lock != NULL) {
         /* Unlock the lock so it's safe to free it */
         if (self->locked)
@@ -48,12 +52,13 @@ acquire_timed(PyThread_type_lock lock, _PyTime_t timeout)
 {
     PyLockStatus r;
     _PyTime_t endtime = 0;
-    _PyTime_t microseconds;
 
-    if (timeout > 0)
+    if (timeout > 0) {
         endtime = _PyTime_GetMonotonicClock() + timeout;
+    }
 
     do {
+        _PyTime_t microseconds;
         microseconds = _PyTime_AsMicroseconds(timeout, _PyTime_ROUND_CEILING);
 
         /* first a simple non-blocking try without releasing the GIL */
@@ -138,12 +143,10 @@ static PyObject *
 lock_PyThread_acquire_lock(lockobject *self, PyObject *args, PyObject *kwds)
 {
     _PyTime_t timeout;
-    PyLockStatus r;
-
     if (lock_acquire_parse_args(args, kwds, &timeout) < 0)
         return NULL;
 
-    r = acquire_timed(self->lock_lock, timeout);
+    PyLockStatus r = acquire_timed(self->lock_lock, timeout);
     if (r == PY_LOCK_INTR) {
         return NULL;
     }
@@ -245,36 +248,28 @@ static PyMethodDef lock_methods[] = {
     {NULL,           NULL}              /* sentinel */
 };
 
+PyDoc_STRVAR(lock_doc,
+"A lock object is a synchronization primitive.  To create a lock,\n\
+call threading.Lock().  Methods are:\n\
+\n\
+acquire() -- lock the lock, possibly blocking until it can be obtained\n\
+release() -- unlock of the lock\n\
+locked() -- test whether the lock is currently locked\n\
+\n\
+A lock is not owned by the thread that locked it; another thread may\n\
+unlock it.  A thread attempting to lock a lock that it has already locked\n\
+will block until another thread unlocks it.  Deadlocks may ensue.");
+
 static PyTypeObject Locktype = {
     PyVarObject_HEAD_INIT(&PyType_Type, 0)
-    "_thread.lock",                     /*tp_name*/
-    sizeof(lockobject),                 /*tp_basicsize*/
-    0,                                  /*tp_itemsize*/
-    /* methods */
-    (destructor)lock_dealloc,           /*tp_dealloc*/
-    0,                                  /*tp_vectorcall_offset*/
-    0,                                  /*tp_getattr*/
-    0,                                  /*tp_setattr*/
-    0,                                  /*tp_as_async*/
-    (reprfunc)lock_repr,                /*tp_repr*/
-    0,                                  /*tp_as_number*/
-    0,                                  /*tp_as_sequence*/
-    0,                                  /*tp_as_mapping*/
-    0,                                  /*tp_hash*/
-    0,                                  /*tp_call*/
-    0,                                  /*tp_str*/
-    0,                                  /*tp_getattro*/
-    0,                                  /*tp_setattro*/
-    0,                                  /*tp_as_buffer*/
-    Py_TPFLAGS_DEFAULT,                 /*tp_flags*/
-    0,                                  /*tp_doc*/
-    0,                                  /*tp_traverse*/
-    0,                                  /*tp_clear*/
-    0,                                  /*tp_richcompare*/
-    offsetof(lockobject, in_weakreflist), /*tp_weaklistoffset*/
-    0,                                  /*tp_iter*/
-    0,                                  /*tp_iternext*/
-    lock_methods,                       /*tp_methods*/
+    .tp_name = "_thread.lock",
+    .tp_basicsize = sizeof(lockobject),
+    .tp_dealloc = (destructor)lock_dealloc,
+    .tp_repr = (reprfunc)lock_repr,
+    .tp_flags = Py_TPFLAGS_DEFAULT,
+    .tp_doc = lock_doc,
+    .tp_weaklistoffset = offsetof(lockobject, in_weakreflist),
+    .tp_methods = lock_methods,
 };
 
 /* Recursive lock objects */
@@ -527,56 +522,29 @@ static PyMethodDef rlock_methods[] = {
 
 static PyTypeObject RLocktype = {
     PyVarObject_HEAD_INIT(&PyType_Type, 0)
-    "_thread.RLock",                    /*tp_name*/
-    sizeof(rlockobject),                /*tp_basicsize*/
-    0,                                  /*tp_itemsize*/
-    /* methods */
-    (destructor)rlock_dealloc,          /*tp_dealloc*/
-    0,                                  /*tp_vectorcall_offset*/
-    0,                                  /*tp_getattr*/
-    0,                                  /*tp_setattr*/
-    0,                                  /*tp_as_async*/
-    (reprfunc)rlock_repr,               /*tp_repr*/
-    0,                                  /*tp_as_number*/
-    0,                                  /*tp_as_sequence*/
-    0,                                  /*tp_as_mapping*/
-    0,                                  /*tp_hash*/
-    0,                                  /*tp_call*/
-    0,                                  /*tp_str*/
-    0,                                  /*tp_getattro*/
-    0,                                  /*tp_setattro*/
-    0,                                  /*tp_as_buffer*/
-    Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */
-    0,                                  /*tp_doc*/
-    0,                                  /*tp_traverse*/
-    0,                                  /*tp_clear*/
-    0,                                  /*tp_richcompare*/
-    offsetof(rlockobject, in_weakreflist), /*tp_weaklistoffset*/
-    0,                                  /*tp_iter*/
-    0,                                  /*tp_iternext*/
-    rlock_methods,                      /*tp_methods*/
-    0,                                  /* tp_members */
-    0,                                  /* tp_getset */
-    0,                                  /* tp_base */
-    0,                                  /* tp_dict */
-    0,                                  /* tp_descr_get */
-    0,                                  /* tp_descr_set */
-    0,                                  /* tp_dictoffset */
-    0,                                  /* tp_init */
-    PyType_GenericAlloc,                /* tp_alloc */
-    rlock_new                           /* tp_new */
+    .tp_name = "_thread.RLock",
+    .tp_basicsize = sizeof(rlockobject),
+    .tp_dealloc = (destructor)rlock_dealloc,
+    .tp_repr = (reprfunc)rlock_repr,
+    .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
+    .tp_weaklistoffset = offsetof(rlockobject, in_weakreflist),
+    .tp_methods = rlock_methods,
+    .tp_alloc = PyType_GenericAlloc,
+    .tp_new = rlock_new,
 };
 
 static lockobject *
 newlockobject(void)
 {
-    lockobject *self;
-    self = PyObject_New(lockobject, &Locktype);
-    if (self == NULL)
+    lockobject *self = PyObject_New(lockobject, &Locktype);
+    if (self == NULL) {
         return NULL;
+    }
+
     self->lock_lock = PyThread_allocate_lock();
     self->locked = 0;
     self->in_weakreflist = NULL;
+
     if (self->lock_lock == NULL) {
         Py_DECREF(self);
         PyErr_SetString(ThreadError, "can't allocate lock");
@@ -642,30 +610,12 @@ localdummy_dealloc(localdummyobject *self)
 
 static PyTypeObject localdummytype = {
     PyVarObject_HEAD_INIT(NULL, 0)
-    /* tp_name           */ "_thread._localdummy",
-    /* tp_basicsize      */ sizeof(localdummyobject),
-    /* tp_itemsize       */ 0,
-    /* tp_dealloc        */ (destructor)localdummy_dealloc,
-    /* tp_vectorcall_offset */ 0,
-    /* tp_getattr        */ 0,
-    /* tp_setattr        */ 0,
-    /* tp_as_async       */ 0,
-    /* tp_repr           */ 0,
-    /* tp_as_number      */ 0,
-    /* tp_as_sequence    */ 0,
-    /* tp_as_mapping     */ 0,
-    /* tp_hash           */ 0,
-    /* tp_call           */ 0,
-    /* tp_str            */ 0,
-    /* tp_getattro       */ 0,
-    /* tp_setattro       */ 0,
-    /* tp_as_buffer      */ 0,
-    /* tp_flags          */ Py_TPFLAGS_DEFAULT,
-    /* tp_doc            */ "Thread-local dummy",
-    /* tp_traverse       */ 0,
-    /* tp_clear          */ 0,
-    /* tp_richcompare    */ 0,
-    /* tp_weaklistoffset */ offsetof(localdummyobject, weakreflist)
+    .tp_name = "_thread._localdummy",
+    .tp_basicsize = sizeof(localdummyobject),
+    .tp_dealloc = (destructor)localdummy_dealloc,
+    .tp_flags = Py_TPFLAGS_DEFAULT,
+    .tp_doc = "Thread-local dummy",
+    .tp_weaklistoffset = offsetof(localdummyobject, weakreflist),
 };
 
 
@@ -690,11 +640,10 @@ static PyObject *_localdummy_destroyed(PyObject *meth_self, PyObject *dummyweakr
 static PyObject *
 _local_create_dummy(localobject *self)
 {
-    PyObject *tdict, *ldict = NULL, *wr = NULL;
+    PyObject *ldict = NULL, *wr = NULL;
     localdummyobject *dummy = NULL;
-    int r;
 
-    tdict = PyThreadState_GetDict();
+    PyObject *tdict = PyThreadState_GetDict();
     if (tdict == NULL) {
         PyErr_SetString(PyExc_SystemError,
                         "Couldn't get thread-state dictionary");
@@ -702,25 +651,30 @@ _local_create_dummy(localobject *self)
     }
 
     ldict = PyDict_New();
-    if (ldict == NULL)
+    if (ldict == NULL) {
         goto err;
+    }
     dummy = (localdummyobject *) localdummytype.tp_alloc(&localdummytype, 0);
-    if (dummy == NULL)
+    if (dummy == NULL) {
         goto err;
+    }
     dummy->localdict = ldict;
     wr = PyWeakref_NewRef((PyObject *) dummy, self->wr_callback);
-    if (wr == NULL)
+    if (wr == NULL) {
         goto err;
+    }
 
     /* As a side-effect, this will cache the weakref's hash before the
        dummy gets deleted */
-    r = PyDict_SetItem(self->dummies, wr, ldict);
-    if (r < 0)
+    int r = PyDict_SetItem(self->dummies, wr, ldict);
+    if (r < 0) {
         goto err;
+    }
     Py_CLEAR(wr);
     r = PyDict_SetItem(tdict, self->key, (PyObject *) dummy);
-    if (r < 0)
+    if (r < 0) {
         goto err;
+    }
     Py_CLEAR(dummy);
 
     Py_DECREF(ldict);
@@ -737,7 +691,6 @@ static PyObject *
 local_new(PyTypeObject *type, PyObject *args, PyObject *kw)
 {
     localobject *self;
-    PyObject *wr;
     static PyMethodDef wr_callback_def = {
         "_localdummy_destroyed", (PyCFunction) _localdummy_destroyed, METH_O
     };
@@ -749,42 +702,45 @@ local_new(PyTypeObject *type, PyObject *args, PyObject *kw)
         if (rc == 0 && kw != NULL)
             rc = PyObject_IsTrue(kw);
         if (rc != 0) {
-            if (rc > 0)
+            if (rc > 0) {
                 PyErr_SetString(PyExc_TypeError,
                           "Initialization arguments are not supported");
+            }
             return NULL;
         }
     }
 
     self = (localobject *)type->tp_alloc(type, 0);
-    if (self == NULL)
+    if (self == NULL) {
         return NULL;
+    }
 
-    Py_XINCREF(args);
-    self->args = args;
-    Py_XINCREF(kw);
-    self->kw = kw;
+    self->args = Py_XNewRef(args);
+    self->kw = Py_XNewRef(kw);
     self->key = PyUnicode_FromFormat("thread.local.%p", self);
-    if (self->key == NULL)
+    if (self->key == NULL) {
         goto err;
+    }
 
     self->dummies = PyDict_New();
-    if (self->dummies == NULL)
+    if (self->dummies == NULL) {
         goto err;
+    }
 
     /* We use a weak reference to self in the callback closure
        in order to avoid spurious reference cycles */
-    wr = PyWeakref_NewRef((PyObject *) self, NULL);
-    if (wr == NULL)
+    PyObject *wr = PyWeakref_NewRef((PyObject *) self, NULL);
+    if (wr == NULL) {
         goto err;
+    }
     self->wr_callback = PyCFunction_NewEx(&wr_callback_def, wr, NULL);
     Py_DECREF(wr);
-    if (self->wr_callback == NULL)
+    if (self->wr_callback == NULL) {
         goto err;
-
-    if (_local_create_dummy(self) == NULL)
+    }
+    if (_local_create_dummy(self) == NULL) {
         goto err;
-
+    }
     return (PyObject *)self;
 
   err:
@@ -834,8 +790,9 @@ local_dealloc(localobject *self)
 {
     /* Weakrefs must be invalidated right now, otherwise they can be used
        from code called below, which is very dangerous since Py_REFCNT(self) == 0 */
-    if (self->weakreflist != NULL)
+    if (self->weakreflist != NULL) {
         PyObject_ClearWeakRefs((PyObject *) self);
+    }
 
     PyObject_GC_UnTrack(self);
 
@@ -848,16 +805,15 @@ local_dealloc(localobject *self)
 static PyObject *
 _ldict(localobject *self)
 {
-    PyObject *tdict, *ldict, *dummy;
-
-    tdict = PyThreadState_GetDict();
+    PyObject *tdict = PyThreadState_GetDict();
     if (tdict == NULL) {
         PyErr_SetString(PyExc_SystemError,
                         "Couldn't get thread-state dictionary");
         return NULL;
     }
 
-    dummy = PyDict_GetItemWithError(tdict, self->key);
+    PyObject *ldict;
+    PyObject *dummy = PyDict_GetItemWithError(tdict, self->key);
     if (dummy == NULL) {
         if (PyErr_Occurred()) {
             return NULL;
@@ -887,22 +843,26 @@ _ldict(localobject *self)
 static int
 local_setattro(localobject *self, PyObject *name, PyObject *v)
 {
-    PyObject *ldict;
-    int r;
+    PyObject *ldict = _ldict(self);
+    if (ldict == NULL) {
+        return -1;
+    }
 
-    ldict = _ldict(self);
-    if (ldict == NULL)
+    PyObject *str_dict = _PyUnicode_FromId(&PyId___dict__);  // borrowed ref
+    if (str_dict == NULL) {
         return -1;
+    }
 
-    r = PyObject_RichCompareBool(name, str_dict, Py_EQ);
+    int r = PyObject_RichCompareBool(name, str_dict, Py_EQ);
+    if (r == -1) {
+        return -1;
+    }
     if (r == 1) {
         PyErr_Format(PyExc_AttributeError,
                      "'%.50s' object attribute '%U' is read-only",
                      Py_TYPE(self)->tp_name, name);
         return -1;
     }
-    if (r == -1)
-        return -1;
 
     return _PyObject_GenericSetAttrWithDict((PyObject *)self, name, v, ldict);
 }
@@ -911,80 +871,54 @@ static PyObject *local_getattro(localobject *, PyObject *);
 
 static PyTypeObject localtype = {
     PyVarObject_HEAD_INIT(NULL, 0)
-    /* tp_name           */ "_thread._local",
-    /* tp_basicsize      */ sizeof(localobject),
-    /* tp_itemsize       */ 0,
-    /* tp_dealloc        */ (destructor)local_dealloc,
-    /* tp_vectorcall_offset */ 0,
-    /* tp_getattr        */ 0,
-    /* tp_setattr        */ 0,
-    /* tp_as_async       */ 0,
-    /* tp_repr           */ 0,
-    /* tp_as_number      */ 0,
-    /* tp_as_sequence    */ 0,
-    /* tp_as_mapping     */ 0,
-    /* tp_hash           */ 0,
-    /* tp_call           */ 0,
-    /* tp_str            */ 0,
-    /* tp_getattro       */ (getattrofunc)local_getattro,
-    /* tp_setattro       */ (setattrofunc)local_setattro,
-    /* tp_as_buffer      */ 0,
-    /* tp_flags          */ Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE
-                                               | Py_TPFLAGS_HAVE_GC,
-    /* tp_doc            */ "Thread-local data",
-    /* tp_traverse       */ (traverseproc)local_traverse,
-    /* tp_clear          */ (inquiry)local_clear,
-    /* tp_richcompare    */ 0,
-    /* tp_weaklistoffset */ offsetof(localobject, weakreflist),
-    /* tp_iter           */ 0,
-    /* tp_iternext       */ 0,
-    /* tp_methods        */ 0,
-    /* tp_members        */ 0,
-    /* tp_getset         */ 0,
-    /* tp_base           */ 0,
-    /* tp_dict           */ 0, /* internal use */
-    /* tp_descr_get      */ 0,
-    /* tp_descr_set      */ 0,
-    /* tp_dictoffset     */ 0,
-    /* tp_init           */ 0,
-    /* tp_alloc          */ 0,
-    /* tp_new            */ local_new,
-    /* tp_free           */ 0, /* Low-level free-mem routine */
-    /* tp_is_gc          */ 0, /* For PyObject_IS_GC */
+    .tp_name = "_thread._local",
+    .tp_basicsize = sizeof(localobject),
+    .tp_dealloc = (destructor)local_dealloc,
+    .tp_getattro = (getattrofunc)local_getattro,
+    .tp_setattro = (setattrofunc)local_setattro,
+    .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC,
+    .tp_doc = "Thread-local data",
+    .tp_traverse = (traverseproc)local_traverse,
+    .tp_clear = (inquiry)local_clear,
+    .tp_weaklistoffset = offsetof(localobject, weakreflist),
+    .tp_new = local_new,
 };
 
 static PyObject *
 local_getattro(localobject *self, PyObject *name)
 {
-    PyObject *ldict, *value;
-    int r;
-
-    ldict = _ldict(self);
+    PyObject *ldict = _ldict(self);
     if (ldict == NULL)
         return NULL;
 
-    r = PyObject_RichCompareBool(name, str_dict, Py_EQ);
+    PyObject *str_dict = _PyUnicode_FromId(&PyId___dict__);  // borrowed ref
+    if (str_dict == NULL) {
+        return NULL;
+    }
+
+    int r = PyObject_RichCompareBool(name, str_dict, Py_EQ);
     if (r == 1) {
-        Py_INCREF(ldict);
-        return ldict;
+        return Py_NewRef(ldict);
     }
-    if (r == -1)
+    if (r == -1) {
         return NULL;
+    }
 
-    if (!Py_IS_TYPE(self, &localtype))
+    if (!Py_IS_TYPE(self, &localtype)) {
         /* use generic lookup for subtypes */
-        return _PyObject_GenericGetAttrWithDict(
-            (PyObject *)self, name, ldict, 0);
+        return _PyObject_GenericGetAttrWithDict((PyObject *)self, name,
+                                                ldict, 0);
+    }
 
     /* Optimization: just look in dict ourselves */
-    value = PyDict_GetItemWithError(ldict, name);
+    PyObject *value = PyDict_GetItemWithError(ldict, name);
     if (value != NULL) {
-        Py_INCREF(value);
-        return value;
+        return Py_NewRef(value);
     }
-    else if (PyErr_Occurred()) {
+    if (PyErr_Occurred()) {
         return NULL;
     }
+
     /* Fall back on generic to get __class__ and __dict__ */
     return _PyObject_GenericGetAttrWithDict(
         (PyObject *)self, name, ldict, 0);
@@ -994,17 +928,15 @@ local_getattro(localobject *self, PyObject *name)
 static PyObject *
 _localdummy_destroyed(PyObject *localweakref, PyObject *dummyweakref)
 {
-    PyObject *obj;
-    localobject *self;
     assert(PyWeakref_CheckRef(localweakref));
-    obj = PyWeakref_GET_OBJECT(localweakref);
-    if (obj == Py_None)
+    PyObject *obj = PyWeakref_GET_OBJECT(localweakref);
+    if (obj == Py_None) {
         Py_RETURN_NONE;
-    Py_INCREF(obj);
-    assert(PyObject_TypeCheck(obj, &localtype));
+    }
+
     /* If the thread-local object is still alive and not being cleared,
        remove the corresponding local dict */
-    self = (localobject *) obj;
+    localobject *self = (localobject *)Py_NewRef(obj);
     if (self->dummies != NULL) {
         PyObject *ldict;
         ldict = PyDict_GetItemWithError(self->dummies, dummyweakref);
@@ -1024,24 +956,35 @@ struct bootstate {
     PyInterpreterState *interp;
     PyObject *func;
     PyObject *args;
-    PyObject *keyw;
+    PyObject *kwargs;
     PyThreadState *tstate;
     _PyRuntimeState *runtime;
 };
 
+
 static void
-t_bootstrap(void *boot_raw)
+thread_bootstate_free(struct bootstate *boot)
+{
+    Py_DECREF(boot->func);
+    Py_DECREF(boot->args);
+    Py_XDECREF(boot->kwargs);
+    PyMem_Free(boot);
+}
+
+
+static void
+thread_run(void *boot_raw)
 {
     struct bootstate *boot = (struct bootstate *) boot_raw;
     PyThreadState *tstate;
-    PyObject *res;
 
     tstate = boot->tstate;
     tstate->thread_id = PyThread_get_thread_ident();
     _PyThreadState_Init(tstate);
     PyEval_AcquireThread(tstate);
     tstate->interp->num_threads++;
-    res = PyObject_Call(boot->func, boot->args, boot->keyw);
+
+    PyObject *res = PyObject_Call(boot->func, boot->args, boot->kwargs);
     if (res == NULL) {
         if (PyErr_ExceptionMatches(PyExc_SystemExit))
             /* SystemExit is ignored silently */
@@ -1053,13 +996,12 @@ t_bootstrap(void *boot_raw)
     else {
         Py_DECREF(res);
     }
-    Py_DECREF(boot->func);
-    Py_DECREF(boot->args);
-    Py_XDECREF(boot->keyw);
-    PyMem_Free(boot_raw);
+
+    thread_bootstate_free(boot);
     tstate->interp->num_threads--;
     PyThreadState_Clear(tstate);
     _PyThreadState_DeleteCurrent(tstate);
+
     PyThread_exit_thread();
 }
 
@@ -1067,12 +1009,10 @@ static PyObject *
 thread_PyThread_start_new_thread(PyObject *self, PyObject *fargs)
 {
     _PyRuntimeState *runtime = &_PyRuntime;
-    PyObject *func, *args, *keyw = NULL;
-    struct bootstate *boot;
-    unsigned long ident;
+    PyObject *func, *args, *kwargs = NULL;
 
     if (!PyArg_UnpackTuple(fargs, "start_new_thread", 2, 3,
-                           &func, &args, &keyw))
+                           &func, &args, &kwargs))
         return NULL;
     if (!PyCallable_Check(func)) {
         PyErr_SetString(PyExc_TypeError,
@@ -1084,7 +1024,7 @@ thread_PyThread_start_new_thread(PyObject *self, PyObject *fargs)
                         "2nd arg must be a tuple");
         return NULL;
     }
-    if (keyw != NULL && !PyDict_Check(keyw)) {
+    if (kwargs != NULL && !PyDict_Check(kwargs)) {
         PyErr_SetString(PyExc_TypeError,
                         "optional 3rd arg must be a dictionary");
         return NULL;
@@ -1097,31 +1037,26 @@ thread_PyThread_start_new_thread(PyObject *self, PyObject *fargs)
         return NULL;
     }
 
-    boot = PyMem_NEW(struct bootstate, 1);
-    if (boot == NULL)
+    struct bootstate *boot = PyMem_NEW(struct bootstate, 1);
+    if (boot == NULL) {
         return PyErr_NoMemory();
+    }
     boot->interp = _PyInterpreterState_GET();
-    boot->func = func;
-    boot->args = args;
-    boot->keyw = keyw;
     boot->tstate = _PyThreadState_Prealloc(boot->interp);
-    boot->runtime = runtime;
     if (boot->tstate == NULL) {
         PyMem_Free(boot);
         return PyErr_NoMemory();
     }
-    Py_INCREF(func);
-    Py_INCREF(args);
-    Py_XINCREF(keyw);
+    boot->runtime = runtime;
+    boot->func = Py_NewRef(func);
+    boot->args = Py_NewRef(args);
+    boot->kwargs = Py_XNewRef(kwargs);
 
-    ident = PyThread_start_new_thread(t_bootstrap, (void*) boot);
+    unsigned long ident = PyThread_start_new_thread(thread_run, (void*) boot);
     if (ident == PYTHREAD_INVALID_THREAD_ID) {
         PyErr_SetString(ThreadError, "can't start new thread");
-        Py_DECREF(func);
-        Py_DECREF(args);
-        Py_XDECREF(keyw);
         PyThreadState_Clear(boot->tstate);
-        PyMem_Free(boot);
+        thread_bootstate_free(boot);
         return NULL;
     }
     return PyLong_FromUnsignedLong(ident);
@@ -1169,7 +1104,7 @@ A subthread can use this function to interrupt the main thread."
 static lockobject *newlockobject(void);
 
 static PyObject *
-thread_PyThread_allocate_lock(PyObject *self, PyObject *Py_UNUSED(ignored))
+thread_PyThread_allocate_lock(PyObject *module, PyObject *Py_UNUSED(ignored))
 {
     return (PyObject *) newlockobject();
 }
@@ -1248,7 +1183,6 @@ release_sentinel(void *wr_raw)
     PyObject *obj = PyWeakref_GET_OBJECT(wr);
     lockobject *lock;
     if (obj != Py_None) {
-        assert(Py_IS_TYPE(obj, &Locktype));
         lock = (lockobject *) obj;
         if (lock->locked) {
             PyThread_release_lock(lock->lock_lock);
@@ -1261,7 +1195,7 @@ release_sentinel(void *wr_raw)
 }
 
 static PyObject *
-thread__set_sentinel(PyObject *self, PyObject *Py_UNUSED(ignored))
+thread__set_sentinel(PyObject *module, PyObject *Py_UNUSED(ignored))
 {
     PyObject *wr;
     PyThreadState *tstate = PyThreadState_Get();
@@ -1429,7 +1363,7 @@ static PyStructSequence_Field ExceptHookArgs_fields[] = {
 };
 
 static PyStructSequence_Desc ExceptHookArgs_desc = {
-    .name = "_thread.ExceptHookArgs",
+    .name = "_thread._ExceptHookArgs",
     .doc = ExceptHookArgs__doc__,
     .fields = ExceptHookArgs_fields,
     .n_in_sequence = 4
@@ -1530,106 +1464,87 @@ static PyMethodDef thread_methods[] = {
 
 /* Initialization function */
 
-PyDoc_STRVAR(thread_doc,
-"This module provides primitive operations to write multi-threaded programs.\n\
-The 'threading' module provides a more convenient interface.");
-
-PyDoc_STRVAR(lock_doc,
-"A lock object is a synchronization primitive.  To create a lock,\n\
-call threading.Lock().  Methods are:\n\
-\n\
-acquire() -- lock the lock, possibly blocking until it can be obtained\n\
-release() -- unlock of the lock\n\
-locked() -- test whether the lock is currently locked\n\
-\n\
-A lock is not owned by the thread that locked it; another thread may\n\
-unlock it.  A thread attempting to lock a lock that it has already locked\n\
-will block until another thread unlocks it.  Deadlocks may ensue.");
-
-static struct PyModuleDef threadmodule = {
-    PyModuleDef_HEAD_INIT,
-    "_thread",
-    thread_doc,
-    -1,
-    thread_methods,
-    NULL,
-    NULL,
-    NULL,
-    NULL
-};
-
-
-PyMODINIT_FUNC
-PyInit__thread(void)
+static int
+_thread_module_exec(PyObject *module)
 {
-    PyObject *m, *d, *v;
-    double time_max;
-    double timeout_max;
-    PyInterpreterState *interp = _PyInterpreterState_GET();
+    // Initialize the C thread library
+    PyThread_init_thread();
 
-    /* Initialize types: */
+    // Initialize types
     if (PyType_Ready(&localdummytype) < 0)
-        return NULL;
-    if (PyType_Ready(&localtype) < 0)
-        return NULL;
-    if (PyType_Ready(&Locktype) < 0)
-        return NULL;
-    if (PyType_Ready(&RLocktype) < 0)
-        return NULL;
+        return -1;
+    if (PyType_Ready(&localtype) < 0) {
+        return -1;
+    }
+    if (PyType_Ready(&Locktype) < 0) {
+        return -1;
+    }
+    if (PyType_Ready(&RLocktype) < 0) {
+        return -1;
+    }
     if (ExceptHookArgsType.tp_name == NULL) {
         if (PyStructSequence_InitType2(&ExceptHookArgsType,
                                        &ExceptHookArgs_desc) < 0) {
-            return NULL;
+            return -1;
         }
     }
 
-    /* Create the module and add the functions */
-    m = PyModule_Create(&threadmodule);
-    if (m == NULL)
-        return NULL;
+    // Add module attributes
+    PyObject *d = PyModule_GetDict(module);
+    if (PyDict_SetItemString(d, "error", ThreadError) < 0) {
+        return -1;
+    }
+    if (PyDict_SetItemString(d, "LockType", (PyObject *)&Locktype) < 0) {
+        return -1;
+    }
+    if (PyModule_AddType(module, &RLocktype) < 0) {
+        return -1;
+    }
+    if (PyModule_AddType(module, &localtype) < 0) {
+        return -1;
+    }
+    if (PyModule_AddType(module, &ExceptHookArgsType) < 0) {
+        return -1;
+    }
 
-    timeout_max = (_PyTime_t)PY_TIMEOUT_MAX * 1e-6;
-    time_max = _PyTime_AsSecondsDouble(_PyTime_MAX);
+    // TIMEOUT_MAX
+    double timeout_max = (_PyTime_t)PY_TIMEOUT_MAX * 1e-6;
+    double time_max = _PyTime_AsSecondsDouble(_PyTime_MAX);
     timeout_max = Py_MIN(timeout_max, time_max);
-    /* Round towards minus infinity */
+    // Round towards minus infinity
     timeout_max = floor(timeout_max);
 
-    v = PyFloat_FromDouble(timeout_max);
-    if (!v)
-        return NULL;
-    if (PyModule_AddObject(m, "TIMEOUT_MAX", v) < 0)
-        return NULL;
+    if (PyModule_AddObject(module, "TIMEOUT_MAX",
+                           PyFloat_FromDouble(timeout_max)) < 0) {
+        return -1;
+    }
 
-    /* Add a symbolic constant */
-    d = PyModule_GetDict(m);
-    ThreadError = PyExc_RuntimeError;
-    Py_INCREF(ThreadError);
+    return 0;
+}
 
-    PyDict_SetItemString(d, "error", ThreadError);
-    Locktype.tp_doc = lock_doc;
-    Py_INCREF(&Locktype);
-    PyDict_SetItemString(d, "LockType", (PyObject *)&Locktype);
 
-    Py_INCREF(&RLocktype);
-    if (PyModule_AddObject(m, "RLock", (PyObject *)&RLocktype) < 0)
-        return NULL;
+PyDoc_STRVAR(thread_doc,
+"This module provides primitive operations to write multi-threaded programs.\n\
+The 'threading' module provides a more convenient interface.");
 
-    Py_INCREF(&localtype);
-    if (PyModule_AddObject(m, "_local", (PyObject *)&localtype) < 0)
-        return NULL;
+static struct PyModuleDef _thread_module = {
+    PyModuleDef_HEAD_INIT,
+    .m_name = "_thread",
+    .m_doc = thread_doc,
+    .m_size = -1,
+    .m_methods = thread_methods,
+};
 
-    Py_INCREF(&ExceptHookArgsType);
-    if (PyModule_AddObject(m, "_ExceptHookArgs",
-                           (PyObject *)&ExceptHookArgsType) < 0)
+PyMODINIT_FUNC
+PyInit__thread(void)
+{
+    PyObject *module = PyModule_Create(&_thread_module);
+    if (module == NULL)
         return NULL;
 
-    interp->num_threads = 0;
-
-    str_dict = PyUnicode_InternFromString("__dict__");
-    if (str_dict == NULL)
+    if (_thread_module_exec(module) < 0) {
+        Py_DECREF(module);
         return NULL;
-
-    /* Initialize the C thread library */
-    PyThread_init_thread();
-    return m;
+    }
+    return module;
 }