Commit ec637745 authored by Kevin Modzelewski's avatar Kevin Modzelewski

Fix a bunch of destructor stuff

First, our implementation of slot_tp_del (what calls Python-level __del__ methods)
was pretty bad -- it didn't re-incref the object to keep it alive.

Then we had some issues around running destructors at shutdown.  I made this section
look a bit more like CPython's, but there are still differences.
parent 4c59b7c7
...@@ -136,6 +136,9 @@ PyAPI_FUNC(void) PyType_SetDict(PyTypeObject*, PyObject*) PYSTON_NOEXCEPT; ...@@ -136,6 +136,9 @@ PyAPI_FUNC(void) PyType_SetDict(PyTypeObject*, PyObject*) PYSTON_NOEXCEPT;
// PyType_Ready calls this automatically. // PyType_Ready calls this automatically.
PyAPI_FUNC(PyObject*) PyGC_RegisterStaticConstant(PyObject*) PYSTON_NOEXCEPT; PyAPI_FUNC(PyObject*) PyGC_RegisterStaticConstant(PyObject*) PYSTON_NOEXCEPT;
// Gets gc.garbage
PyAPI_FUNC(PyObject*) _PyGC_GetGarbage() PYSTON_NOEXCEPT;
// Pyston addition: // Pyston addition:
PyAPI_FUNC(void) PyGC_Enable() PYSTON_NOEXCEPT; PyAPI_FUNC(void) PyGC_Enable() PYSTON_NOEXCEPT;
PyAPI_FUNC(void) PyGC_Disable() PYSTON_NOEXCEPT; PyAPI_FUNC(void) PyGC_Disable() PYSTON_NOEXCEPT;
......
...@@ -777,6 +777,15 @@ debug_cycle(char *msg, PyObject *op) ...@@ -777,6 +777,15 @@ debug_cycle(char *msg, PyObject *op)
} }
} }
PyObject* _PyGC_GetGarbage() {
if (garbage == NULL) {
garbage = PyList_New(0);
if (garbage == NULL)
Py_FatalError("gc couldn't create gc.garbage list");
}
return garbage;
}
/* Handle uncollectable garbage (cycles with finalizers, and stuff reachable /* Handle uncollectable garbage (cycles with finalizers, and stuff reachable
* only from such cycles). * only from such cycles).
* If DEBUG_SAVEALL, all objects in finalizers are appended to the module * If DEBUG_SAVEALL, all objects in finalizers are appended to the module
...@@ -791,12 +800,9 @@ handle_finalizers(PyGC_Head *finalizers, PyGC_Head *old) ...@@ -791,12 +800,9 @@ handle_finalizers(PyGC_Head *finalizers, PyGC_Head *old)
{ {
PyGC_Head *gc = finalizers->gc.gc_next; PyGC_Head *gc = finalizers->gc.gc_next;
if (garbage == NULL) { if (garbage == NULL)
garbage = PyList_New(0); _PyGC_GetGarbage();
if (garbage == NULL)
Py_FatalError("gc couldn't create gc.garbage list");
PyGC_RegisterStaticConstant(garbage);
}
for (; gc != finalizers; gc = gc->gc.gc_next) { for (; gc != finalizers; gc = gc->gc.gc_next) {
PyObject *op = FROM_GC(gc); PyObject *op = FROM_GC(gc);
......
...@@ -441,15 +441,16 @@ static char* sys_files[] = { ...@@ -441,15 +441,16 @@ static char* sys_files[] = {
/* Un-initialize things, as good as we can */ /* Un-initialize things, as good as we can */
// Pyston change: we don't support calling cleanup currently // Pyston change: we don't support calling cleanup currently
#if 0
void void
PyImport_Cleanup(void) PyImport_Cleanup(void)
{ {
Py_ssize_t pos, ndone; Py_ssize_t pos, ndone;
char *name; char *name;
PyObject *key, *value, *dict; PyObject *key, *value, *dict;
PyInterpreterState *interp = PyThreadState_GET()->interp; // Pyston change:
PyObject *modules = interp->modules; //PyInterpreterState *interp = PyThreadState_GET()->interp;
//PyObject *modules = interp->modules;
PyObject *modules = PySys_GetModulesDict();
if (modules == NULL) if (modules == NULL)
return; /* Already done */ return; /* Already done */
...@@ -568,12 +569,14 @@ PyImport_Cleanup(void) ...@@ -568,12 +569,14 @@ PyImport_Cleanup(void)
/* Finally, clear and delete the modules directory */ /* Finally, clear and delete the modules directory */
PyDict_Clear(modules); PyDict_Clear(modules);
interp->modules = NULL; // Pyston change:
Py_DECREF(modules); //interp->modules = NULL;
Py_CLEAR(interp->modules_reloading); //Py_DECREF(modules);
//Py_CLEAR(interp->modules_reloading);
} }
#if 0
/* Helper for pythonrun.c -- return magic number */ /* Helper for pythonrun.c -- return magic number */
long long
......
...@@ -1117,24 +1117,61 @@ template Box* slotTpGetattrHookInternal<CXX, NOT_REWRITABLE>(Box* self, BoxedStr ...@@ -1117,24 +1117,61 @@ template Box* slotTpGetattrHookInternal<CXX, NOT_REWRITABLE>(Box* self, BoxedStr
} }
} }
static PyObject* slot_tp_del(PyObject* self) noexcept { static void slot_tp_del(PyObject* self) noexcept {
static BoxedString* del_str = getStaticString("__del__"); static PyObject* del_str = NULL;
try { PyObject* del, *res;
// TODO: runtime ICs? PyObject* error_type, *error_value, *error_traceback;
Box* del_attr = typeLookup(self->cls, del_str);
assert(del_attr); /* Temporarily resurrect the object. */
assert(self->ob_refcnt == 0);
CallattrFlags flags{.cls_only = false, self->ob_refcnt = 1;
.null_on_nonexistent = true,
.argspec = ArgPassSpec(0, 0, false, false) }; /* Save the current exception, if any. */
return callattr(self, del_str, flags, NULL, NULL, NULL, NULL, NULL); PyErr_Fetch(&error_type, &error_value, &error_traceback);
} catch (ExcInfo e) {
// Python does not support exceptions thrown inside finalizers. Instead, it just /* Execute __del__ method, if any. */
// prints a warning that an exception was throw to stderr but ignores it. del = lookup_maybe(self, "__del__", &del_str);
setCAPIException(e); if (del != NULL) {
PyErr_WriteUnraisable(self); res = PyEval_CallObject(del, NULL);
return NULL; if (res == NULL)
PyErr_WriteUnraisable(del);
else
Py_DECREF(res);
Py_DECREF(del);
} }
/* Restore the saved exception. */
PyErr_Restore(error_type, error_value, error_traceback);
/* Undo the temporary resurrection; can't use DECREF here, it would
* cause a recursive call.
*/
assert(self->ob_refcnt > 0);
if (--self->ob_refcnt == 0)
return; /* this is the normal path out */
/* __del__ resurrected it! Make it look like the original Py_DECREF
* never happened.
*/
{
Py_ssize_t refcnt = self->ob_refcnt;
_Py_NewReference(self);
self->ob_refcnt = refcnt;
}
assert(!PyType_IS_GC(Py_TYPE(self)) || _Py_AS_GC(self)->gc.gc_refs != _PyGC_REFS_UNTRACKED);
/* If Py_REF_DEBUG, _Py_NewReference bumped _Py_RefTotal, so
* we need to undo that. */
_Py_DEC_REFTOTAL;
/* If Py_TRACE_REFS, _Py_NewReference re-added self to the object
* chain, so no more to do there.
* If COUNT_ALLOCS, the original decref bumped tp_frees, and
* _Py_NewReference bumped tp_allocs: both of those need to be
* undone.
*/
#ifdef COUNT_ALLOCS
--Py_TYPE(self)->tp_frees;
--Py_TYPE(self)->tp_allocs;
#endif
} }
/* Pyston change: static */ int slot_tp_init(PyObject* self, PyObject* args, PyObject* kwds) noexcept { /* Pyston change: static */ int slot_tp_init(PyObject* self, PyObject* args, PyObject* kwds) noexcept {
......
...@@ -650,6 +650,7 @@ public: ...@@ -650,6 +650,7 @@ public:
int traverse(visitproc visit, void* arg) noexcept; int traverse(visitproc visit, void* arg) noexcept;
void clear() noexcept; void clear() noexcept;
void moduleClear() noexcept; // slightly different order of clearing attributes, meant for modules
}; };
static_assert(sizeof(HCAttrs) == sizeof(struct _hcattrs), ""); static_assert(sizeof(HCAttrs) == sizeof(struct _hcattrs), "");
......
...@@ -189,7 +189,8 @@ int handleArg(char code) { ...@@ -189,7 +189,8 @@ int handleArg(char code) {
else if (code == 'q') else if (code == 'q')
GLOBAL_VERBOSITY = 0; GLOBAL_VERBOSITY = 0;
else if (code == 'v') { else if (code == 'v') {
Py_VerboseFlag++; if (GLOBAL_VERBOSITY)
Py_VerboseFlag++;
GLOBAL_VERBOSITY++; GLOBAL_VERBOSITY++;
} else if (code == 'd') } else if (code == 'd')
SHOW_DISASM = true; SHOW_DISASM = true;
......
...@@ -675,7 +675,7 @@ void setupSys() { ...@@ -675,7 +675,7 @@ void setupSys() {
// sys_module is what holds on to all of the other modules: // sys_module is what holds on to all of the other modules:
Py_INCREF(sys_module); Py_INCREF(sys_module);
constants.push_back(sys_module); late_constants.push_back(sys_module);
sys_module->giveAttrBorrowed("modules", sys_modules_dict); sys_module->giveAttrBorrowed("modules", sys_modules_dict);
......
...@@ -310,18 +310,10 @@ extern "C" int PyDict_SetItem(PyObject* mp, PyObject* _key, PyObject* _item) noe ...@@ -310,18 +310,10 @@ extern "C" int PyDict_SetItem(PyObject* mp, PyObject* _key, PyObject* _item) noe
return 0; return 0;
} }
ASSERT(PyDict_Check(mp) || mp->cls == attrwrapper_cls, "%s", getTypeName(mp)); ASSERT(mp->cls == attrwrapper_cls, "%s", getTypeName(mp));
assert(mp);
Box* b = static_cast<Box*>(mp);
Box* key = static_cast<Box*>(_key);
Box* item = static_cast<Box*>(_item);
assert(key);
assert(item);
try { try {
setitem(b, key, item); attrwrapperSet(mp, _key, _item);
} catch (ExcInfo e) { } catch (ExcInfo e) {
setCAPIException(e); setCAPIException(e);
return -1; return -1;
......
...@@ -1241,12 +1241,13 @@ void HCAttrs::clear() noexcept { ...@@ -1241,12 +1241,13 @@ void HCAttrs::clear() noexcept {
if (unlikely(hcls->type == HiddenClass::DICT_BACKED)) { if (unlikely(hcls->type == HiddenClass::DICT_BACKED)) {
Box* d = this->attr_list->attrs[0]; Box* d = this->attr_list->attrs[0];
Py_DECREF(d);
// Skips the attrlist freelist // Skips the attrlist freelist
PyObject_FREE(this->attr_list); PyObject_FREE(this->attr_list);
this->attr_list = NULL; this->attr_list = NULL;
Py_DECREF(d);
return; return;
} }
...@@ -1264,6 +1265,37 @@ void HCAttrs::clear() noexcept { ...@@ -1264,6 +1265,37 @@ void HCAttrs::clear() noexcept {
} }
} }
void HCAttrs::moduleClear() noexcept {
auto hcls = this->hcls;
if (!hcls)
return;
RELEASE_ASSERT(hcls->type == HiddenClass::NORMAL || hcls->type == HiddenClass::SINGLETON, "");
auto attr_list = this->attr_list;
auto attr_list_size = hcls->attributeArraySize();
for (auto&& p : hcls->getStrAttrOffsets()) {
const char* s = p.first->c_str();
if (s[0] == '_' && s[1] != '_') {
int idx = p.second;
Box* b = attr_list->attrs[idx];
attr_list->attrs[idx] = incref(None);
Py_DECREF(b);
}
}
for (auto&& p : hcls->getStrAttrOffsets()) {
const char* s = p.first->c_str();
if (s[0] != '_' || strcmp(s, "__builtins__") != 0) {
int idx = p.second;
Box* b = attr_list->attrs[idx];
attr_list->attrs[idx] = incref(None);
Py_DECREF(b);
}
}
}
void Box::appendNewHCAttr(BORROWED(Box*) new_attr, SetattrRewriteArgs* rewrite_args) { void Box::appendNewHCAttr(BORROWED(Box*) new_attr, SetattrRewriteArgs* rewrite_args) {
assert(cls->instancesHaveHCAttrs()); assert(cls->instancesHaveHCAttrs());
HCAttrs* attrs = getHCAttrsPtr(); HCAttrs* attrs = getHCAttrsPtr();
......
...@@ -2815,6 +2815,12 @@ BORROWED(Box*) unwrapAttrWrapper(Box* b) { ...@@ -2815,6 +2815,12 @@ BORROWED(Box*) unwrapAttrWrapper(Box* b) {
return static_cast<AttrWrapper*>(b)->getUnderlying(); return static_cast<AttrWrapper*>(b)->getUnderlying();
} }
void attrwrapperSet(Box* b, Box* k, Box* v) {
assert(b->cls == attrwrapper_cls);
autoDecref(AttrWrapper::setitem(b, k, v));
}
void Box::setDictBacked(STOLEN(Box*) val) { void Box::setDictBacked(STOLEN(Box*) val) {
// this checks for: v.__dict__ = v.__dict__ // this checks for: v.__dict__ = v.__dict__
...@@ -3815,9 +3821,11 @@ template <typename CM> void clearContiguousMap(CM& cm) { ...@@ -3815,9 +3821,11 @@ template <typename CM> void clearContiguousMap(CM& cm) {
} }
} }
int BoxedModule::clear(Box* b) noexcept { extern "C" void _PyModule_Clear(PyObject* b) noexcept {
BoxedModule* self = static_cast<BoxedModule*>(b); BoxedModule* self = static_cast<BoxedModule*>(b);
self->clearAttrs();
HCAttrs* attrs = self->getHCAttrsPtr();
attrs->moduleClear();
clearContiguousMap(self->str_constants); clearContiguousMap(self->str_constants);
clearContiguousMap(self->unicode_constants); clearContiguousMap(self->unicode_constants);
...@@ -3825,7 +3833,13 @@ int BoxedModule::clear(Box* b) noexcept { ...@@ -3825,7 +3833,13 @@ int BoxedModule::clear(Box* b) noexcept {
clearContiguousMap(self->float_constants); clearContiguousMap(self->float_constants);
clearContiguousMap(self->imaginary_constants); clearContiguousMap(self->imaginary_constants);
clearContiguousMap(self->long_constants); clearContiguousMap(self->long_constants);
assert(!self->keep_alive.size()); assert(!self->keep_alive.size());
}
int BoxedModule::clear(Box* b) noexcept {
_PyModule_Clear(b);
b->clearAttrs();
return 0; return 0;
} }
...@@ -4765,7 +4779,9 @@ extern "C" void Py_Finalize() noexcept { ...@@ -4765,7 +4779,9 @@ extern "C" void Py_Finalize() noexcept {
g.func_addr_registry.dumpPerfMap(); g.func_addr_registry.dumpPerfMap();
call_sys_exitfunc(); call_sys_exitfunc();
// initialized = 0; // initialized = 0;
PyImport_Cleanup();
#ifdef Py_REF_DEBUG #ifdef Py_REF_DEBUG
IN_SHUTDOWN = true; IN_SHUTDOWN = true;
...@@ -4810,6 +4826,16 @@ extern "C" void Py_Finalize() noexcept { ...@@ -4810,6 +4826,16 @@ extern "C" void Py_Finalize() noexcept {
while (PyGC_Collect()) while (PyGC_Collect())
; ;
assert(!constants.size()); assert(!constants.size());
BoxedList* garbage = static_cast<BoxedList*>(_PyGC_GetGarbage());
int num_garbage_objects = garbage->size;
// Free the garbage list, but let all the elements in it stay alive:
for (int i = 0; i < num_garbage_objects; i++) {
Py_INCREF(garbage->elts->elts[i]);
}
Py_DECREF(garbage);
#endif #endif
// PyGC_Collect()); // PyGC_Collect());
...@@ -4868,15 +4894,20 @@ extern "C" void Py_Finalize() noexcept { ...@@ -4868,15 +4894,20 @@ extern "C" void Py_Finalize() noexcept {
teardownCodegen(); teardownCodegen();
#ifdef Py_REF_DEBUG
if (VERBOSITY()) if (VERBOSITY())
PRINT_TOTAL_REFS(); PRINT_TOTAL_REFS();
#ifdef Py_REF_DEBUG
if (num_garbage_objects == 0) {
#ifdef Py_TRACE_REFS #ifdef Py_TRACE_REFS
if (_Py_RefTotal != 0) if (_Py_RefTotal != 0)
_Py_PrintReferenceAddressesCapped(stderr, 10); _Py_PrintReferenceAddressesCapped(stderr, 10);
#endif #endif
RELEASE_ASSERT(_Py_RefTotal == 0, "%ld refs remaining!", _Py_RefTotal); RELEASE_ASSERT(_Py_RefTotal == 0, "%ld refs remaining!", _Py_RefTotal);
} else if (VERBOSITY()) {
fprintf(stderr, "[%d garbage objects]\n", num_garbage_objects);
}
#endif #endif
} }
} }
...@@ -1040,6 +1040,7 @@ public: ...@@ -1040,6 +1040,7 @@ public:
DEFAULT_CLASS(builtin_function_or_method_cls); DEFAULT_CLASS(builtin_function_or_method_cls);
}; };
extern "C" void _PyModule_Clear(PyObject*) noexcept;
class BoxedModule : public Box { class BoxedModule : public Box {
public: public:
HCAttrs attrs; HCAttrs attrs;
...@@ -1076,6 +1077,8 @@ private: ...@@ -1076,6 +1077,8 @@ private:
public: public:
DEFAULT_CLASS(module_cls); DEFAULT_CLASS(module_cls);
friend void _PyModule_Clear(PyObject*) noexcept;
}; };
class BoxedSlice : public Box { class BoxedSlice : public Box {
...@@ -1334,6 +1337,7 @@ Box* attrwrapperKeys(Box* b); ...@@ -1334,6 +1337,7 @@ Box* attrwrapperKeys(Box* b);
void attrwrapperDel(Box* b, llvm::StringRef attr); void attrwrapperDel(Box* b, llvm::StringRef attr);
void attrwrapperClear(Box* b); void attrwrapperClear(Box* b);
BoxedDict* attrwrapperToDict(Box* b); BoxedDict* attrwrapperToDict(Box* b);
void attrwrapperSet(Box* b, Box* k, Box* v);
Box* boxAst(AST* ast); Box* boxAst(AST* ast);
AST* unboxAst(Box* b); AST* unboxAst(Box* b);
......
# expected: reffail
from testing_helpers import test_gc from testing_helpers import test_gc
unordered_finalize = {} unordered_finalize = {}
......
# expected: reffail
# Exceptions from finalizers should get caught: # Exceptions from finalizers should get caught:
import sys import sys
from testing_helpers import test_gc from testing_helpers import test_gc
......
# expected: reffail
import gc import gc
from testing_helpers import test_gc from testing_helpers import test_gc
......
# expected: reffail
from testing_helpers import test_gc from testing_helpers import test_gc
class C(object): class C(object):
......
# expected: reffail
# while I think nothing requires that this works I actually found this in a library... # while I think nothing requires that this works I actually found this in a library...
import subprocess import subprocess
def f(): def f():
......
# expected: reffail
# Objects are allowed to resurrect other objects too, I guess # Objects are allowed to resurrect other objects too, I guess
from testing_helpers import test_gc from testing_helpers import test_gc
......
# expected: fail
# - finalization not implemented yet
# This test might also be broken in the presence of GC
class C(object): class C(object):
pass pass
......
# expected: reffail
# test to ensure that weakref callbacks and finalizers get called in the # test to ensure that weakref callbacks and finalizers get called in the
# right order # right order
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment