Commit 024c246c authored by Kirill Smelkov's avatar Kirill Smelkov

bigfile/py/loadblk: Resort to pybuf unpinninf, if nothing helps

There are situations possible when both exc_traceback and frame objects are
garbage-collected, but frame's f_locals remains not collected because e.g. it
was explicitly added to somewhere. We cannot detect such cases (dicts are not
listed in referrers).

So if nothing helped, as a last resort, unpin pybuf from its original
memory and make it point to zero-sized NULL.

In general this is not strictly correct to do as other buffers &
memoryview objects created from pybuf, copy its pointer on
initialization and thus pybuf unpinning won't adjust them.

However we require BigFile implementations to make sure not to use
such-created objects, if any, after return from loadblk().

Finally fixes nexedi/wendelin.core#7
parent 3a8e1beb
...@@ -125,7 +125,7 @@ static void XPyErr_FullClear(void); ...@@ -125,7 +125,7 @@ static void XPyErr_FullClear(void);
static PyObject* /* PyListObject* */ XPyObject_GetReferrers(PyObject *obj); static PyObject* /* PyListObject* */ XPyObject_GetReferrers(PyObject *obj);
/* print objects that refer to obj */ /* print objects that refer to obj */
static void XPyObject_PrintReferrers(PyObject *obj, FILE *fp); void XPyObject_PrintReferrers(PyObject *obj, FILE *fp);
/* check whether frame f is a callee of top */ /* check whether frame f is a callee of top */
static int XPyFrame_IsCalleeOf(PyFrameObject *f, PyFrameObject *top); static int XPyFrame_IsCalleeOf(PyFrameObject *f, PyFrameObject *top);
...@@ -627,6 +627,37 @@ out: ...@@ -627,6 +627,37 @@ out:
Py_DECREF(pybuf_users); Py_DECREF(pybuf_users);
} }
/* above attempts were "best effort" to unreference pybuf. However we
* cannot completely do so. For example if
* 1. f_locals dict reference pybuf
* 2. f_locals contains only not-tracked by GC keys & values (and pybuf is not tracked)
* 3. frame object for which f_locals was created is already garbage-collected
*
* the f_locals dict won't be even listed in pybuf referrers (python
* dicts and tuples with all atomic types becomes not tracked by GC),
* so we cannot even inspect it.
*
* if nothing helped, as a last resort, unpin pybuf from its original
* memory and make it point to zero-sized NULL.
*
* In general this is not strictly correct to do as other buffers &
* memoryview objects created from pybuf, copy its pointer on
* initialization and thus pybuf unpinning won't adjust them.
*
* However we require BigFile implementations to make sure not to use
* such-created objects, if any, after return from loadblk().
*/
if (pybuf->ob_refcnt != 1) {
#if BIGFILE_USE_OLD_BUFFER
PyBufferObject *pybufo = (PyBufferObject *)pybuf;
XPyBufferObject_Unpin(pybufo);
#else
PyMemoryViewObject *pybufm = (PyMemoryViewObject *)pybuf;
XPyBuffer_Unpin(&pybufm->view);
#endif
}
#if 0
/* now it is real bug if pybuf remains referenced from somewhere */ /* now it is real bug if pybuf remains referenced from somewhere */
if (pybuf->ob_refcnt != 1) { if (pybuf->ob_refcnt != 1) {
WARN("pybuf->ob_refcnt != 1 even after GC:"); WARN("pybuf->ob_refcnt != 1 even after GC:");
...@@ -637,6 +668,7 @@ out: ...@@ -637,6 +668,7 @@ out:
fprintf(stderr, "\n"); fprintf(stderr, "\n");
BUG(); BUG();
} }
#endif
} }
/* drop pybuf /* drop pybuf
...@@ -728,8 +760,10 @@ static int pybigfile_storeblk(BigFile *file, blk_t blk, const void *buf) ...@@ -728,8 +760,10 @@ static int pybigfile_storeblk(BigFile *file, blk_t blk, const void *buf)
* because mbuf.ptr will be a copy of buf.ptr and clearing buf does not * because mbuf.ptr will be a copy of buf.ptr and clearing buf does not
* clear mbuf. * clear mbuf.
* *
* -> TODO rework to call gc.collect() if pybuf->ob_refcnt != 1 * However we require BigFile implementations to make sure not to use
* like loadblk codepath does. */ * such-created objects, if any, after return from storeblk().
*
* See more details in loadblk() codepath */
/* repoint pybuf to empty region - the original memory attached to it can /* repoint pybuf to empty region - the original memory attached to it can
* go away right after we return (if e.g. dirty page was not mapped in any * go away right after we return (if e.g. dirty page was not mapped in any
...@@ -1048,7 +1082,7 @@ XPyObject_GetReferrers(PyObject *obj) ...@@ -1048,7 +1082,7 @@ XPyObject_GetReferrers(PyObject *obj)
return /*(PyListObject *)*/obj_referrers; return /*(PyListObject *)*/obj_referrers;
} }
static void void
XPyObject_PrintReferrers(PyObject *obj, FILE *fp) XPyObject_PrintReferrers(PyObject *obj, FILE *fp)
{ {
PyObject *obj_referrers = XPyObject_GetReferrers(obj); PyObject *obj_referrers = XPyObject_GetReferrers(obj);
......
...@@ -125,6 +125,7 @@ def test_basic(): ...@@ -125,6 +125,7 @@ def test_basic():
# test that python exception state is preserved across pagefaulting # test that python exception state is preserved across pagefaulting
keepg = []
def test_pagefault_savestate(): def test_pagefault_savestate():
keep = [] keep = []
class BadFile(BigFile): class BadFile(BigFile):
...@@ -159,6 +160,9 @@ def test_pagefault_savestate(): ...@@ -159,6 +160,9 @@ def test_pagefault_savestate():
# check same when happenned in function one more level down # check same when happenned in function one more level down
self.func(buf) self.func(buf)
# a case where only f_locals dict is kept alive
self.keep_f_locals(buf)
self.loadblk_run = 1 self.loadblk_run = 1
...@@ -171,6 +175,12 @@ def test_pagefault_savestate(): ...@@ -171,6 +175,12 @@ def test_pagefault_savestate():
assert exc_traceback is not None assert exc_traceback is not None
keep.append(exc_traceback) keep.append(exc_traceback)
@staticmethod
def keep_f_locals(arg):
try:
1/0
except:
keepg.append(sys.exc_info()[2].tb_frame.f_locals)
f = BadFile(PS) f = BadFile(PS)
......
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