Commit 6da5172e authored by Kirill Smelkov's avatar Kirill Smelkov

bigfile/py: Teach storeblk() how to correctly propagate traceback on error

Previously we were limited to printing traceback starting down from just
storeblk() via explicit PyErr_PrintEx() - because pybuf was attached to
memory which could go away right after return from C function - so we
had to destroy that object for sure, not letting any traceback to hold a
reference to it.

This turned out to be too limiting and not showing full context where
errors happen.

So do the following trick: before returning, reattach pybuf to empty
region at NULL, and this way we don't need to worry about pybuf pointing
to memory which can go away -> thus instead of printing exception locally
- just return it the usual way it is done with C api in Python.

NOTE In contrast to PyMemoryViewObject, PyBufferObject definition is not
public, so to support Python2 - had to copy its definition to PY2 compat

NOTE2 loadblk() is not touched - the loading is done from sighandler
context, which simulates as if it work in separate python thread, so it
is leaved as is for now.
parent d53271b9
......@@ -288,9 +288,9 @@ PyFunc(pyfileh_dirty_writeout,
err = fileh_dirty_writeout(pyfileh, flags);
if (err) {
// TODO check if pyerr already occured - just re-raise
// XXX non-informative
PyErr_SetString(PyExc_RuntimeError, "fileh_dirty_writeout fail");
if (!PyErr_Occurred())
// XXX not very informative
PyErr_SetString(PyExc_RuntimeError, "fileh_dirty_writeout fail");
return NULL;
......@@ -588,10 +588,11 @@ err:
static int pybigfile_storeblk(BigFile *file, blk_t blk, const void *buf)
PyBigFile *pyfile = upcast(PyBigFile *, file);
PyObject *pybuf = NULL;
PyObject *pybuf;
PyObject *storeret = NULL;
PyGILState_STATE gstate;
int err;
// XXX so far storeblk() is called only from py/user context (vs SIGSEGV
// context) - no need to save/restore interpreter state.
......@@ -607,37 +608,49 @@ static int pybigfile_storeblk(BigFile *file, blk_t blk, const void *buf)
pybuf = PyMemoryView_FromMemory((void *)buf, file->blksize, PyBUF_READ);
if (!pybuf)
goto err;
goto out;
/* NOTE K = unsigned long long */
BUILD_ASSERT(sizeof(blk) == sizeof(unsigned long long));
storeret = PyObject_CallMethod(pyfile, "storeblk", "KO", blk, pybuf);
if (!storeret)
goto err;
/* we need to know only whether storeret != NULL, decref it now */
/* verify pybuf is not held - its memory can go away right after return
* (if e.g. dirty page was not mapped in any vma) */
if (pybuf)
BUG_ON(pybuf->ob_refcnt != 1);
/* 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
* vma), but we need pybuf to stay not corrupt - for printing full
* traceback in case of storeblk() error. */
PyBufferObject *pybufo = (PyBufferObject *)pybuf;
pybufo->b_ptr = NULL;
pybufo->b_size = 0;
pybufo->b_offset = 0;
pybufo->b_hash = -1;
PyMemoryViewObject *pybufm = (PyMemoryViewObject *)pybuf;
pybufm->view.buf = NULL;
pybufm->view.len = 0;
/* verify that we actually tweaked pybuf ok */
Py_buffer pybuf_view;
err = PyObject_GetBuffer(pybuf, &pybuf_view, PyBUF_SIMPLE);
BUG_ON(pybuf_view.buf != NULL);
BUG_ON(pybuf_view.len != 0);
/* done with pybuf */
/* storeblk() job done */
return storeret ? 0 : -1;
/* error happened - dump traceback and return */
// XXX do we need this in storeblk? (but can't return with pybuf->ob_refcnt
// != 1) and on errors it is not so.
goto out;
......@@ -814,6 +827,11 @@ _init_bigfile(void)
PyObject *m;
int err;
/* verify we copied struct PyBufferObject definition ok */
BUG_ON(sizeof(PyBufferObject) != PyBuffer_Type.tp_basicsize);
/* setup virtmem gil hooks for python */
......@@ -37,4 +37,18 @@ PyMemoryView_FromMemory(char *mem, Py_ssize_t size, int flags)
/* structure of PyBufferObject (Objects/bufferobject.c) */
typedef struct {
PyObject *b_base;
void *b_ptr;
Py_ssize_t b_size;
Py_ssize_t b_offset;
int b_readonly;
long b_hash;
} PyBufferObject;
Markdown is supported
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment