Re: [Libguestfs] Memory corruption when testing nbdkit python plugin with nbd-tester-client?
On 26.09.2016 20:40, Carl-Daniel Hailfinger wrote:
On 26.09.2016 19:20, Carl-Daniel Hailfinger wrote:
> On 26.09.2016 14:29, Richard W.M. Jones wrote:
>> On Mon, Sep 26, 2016 at 02:18:02PM +0200, Carl-Daniel Hailfinger wrote:
>>> Hi,
>>>
>>> has anyone ever run "make check" from nbd against nbdkit with a
python
>>> plugin? I usually get segfaults during such a run, and sometimes various
>>> other errors happen before the segfault, suggesting that some memory
>>> corruption is underway.
>>> AFAICS a pure python plugin should not be able to cause memory corruption.
>> Correct, a python plugin should not cause memory corruption,
>> and nbdkit shouldn't segfault ever.
>>
>> Did you get a stack trace from C (not from Python)?
> The core files were useless, but I ran nbdkit in gdb and got something...
And another one... my close function pickles all python objects which
were accessed in the plugin. Maybe that's what exposes the memory
corruption on close.
[...]
Will try instrumenting the close function next.
The results are in.
I dumped the reference counters for various python objects inside
py_close and often the reference counters are off-by-one or off-by-two
in the crashing case.
AFAICS the python API says that once you have threads, you have to do
the GIL dance:
https://docs.python.org/2/c-api/init.html#thread-state-and-the-global-int...
. All the crashes I saw can be explained by unsynchronized accesses to
python objects.
I found the bug.
plugin_lock_request() is used to lock client requests only, but it does
_not_ lock any other calls into the plugin. Due to that, the following
calls are neither locked against each other nor are they locked against
client requests:
plugin_register (const char *_filename,
plugin_cleanup (void)
plugin_config (const char *key, const char *value)
plugin_config_complete (void)
plugin_open (struct connection *conn, int readonly)
plugin_close (struct connection *conn)
plugin_get_size (struct connection *conn)
plugin_can_write (struct connection *conn)
plugin_can_flush (struct connection *conn)
plugin_is_rotational (struct connection *conn)
plugin_can_trim (struct connection *conn)
That means dumping the plugin-internal data on connection close is not
locked against the requests within that connection, and any garbage
collection triggered by the last request may still be running while
plugin_close runs. Boom.
In general, when working with python any call into python should be
wrapped like this:
PyGILState_STATE gstate;
gstate = PyGILState_Ensure();
/* Perform Python actions here. */
result = CallSomeFunction();
/* evaluate result or handle exception */
/* Release the thread. No Python API allowed beyond this point. */
PyGILState_Release(gstate);
That would even allow us to run everything in parallel.
I have now added a call to plugin_lock_request() within plugin_close()
and will check if it still explodes. That's obviously less than what the
python developers recommend, but it may at least reduce the likelihood
of the crashes.
Regards,
Carl-Daniel