Bug 7902 - Memory problems in pidl python bindings
Summary: Memory problems in pidl python bindings
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: PIDL and libndr (show other bugs)
Version: unspecified
Hardware: Other Linux
: P3 normal (vote)
Target Milestone: ---
Assignee: Samba QA Contact
QA Contact:
URL:
Keywords:
Depends on: 11778
Blocks:
  Show dependency treegraph
 
Reported: 2011-01-06 07:36 UTC by Stefan Metzmacher
Modified: 2019-04-01 21:20 UTC (History)
4 users (show)

See Also:


Attachments
example script (1.42 KB, text/plain)
2011-01-06 07:37 UTC, Stefan Metzmacher
no flags Details
plain output (4.01 KB, text/plain)
2011-01-06 07:38 UTC, Stefan Metzmacher
no flags Details
valgrind output (18.58 KB, text/plain)
2011-01-06 07:38 UTC, Stefan Metzmacher
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Metzmacher 2011-01-06 07:36:07 UTC
$ export PYTHONPATH=bin/python;

$ python -u -tt ../../pidl-python-bug-01.py -u bsddb,network | tee ../../pidl-python-bug-01.plain.txt

$ valgrind --db-attach=no --tool=memcheck --suppressions=valgrind-python.supp python -u -tt ../../pidl-python-bug-01.py -u bsddb,network 2>&1 | tee ../../pidl-python-bug-01.valgrind.txt

See also http://svn.python.org/projects/python/trunk/Misc/README.valgrind
Comment 1 Stefan Metzmacher 2011-01-06 07:37:39 UTC
Created attachment 6183 [details]
example script
Comment 2 Stefan Metzmacher 2011-01-06 07:38:14 UTC
Created attachment 6184 [details]
plain output
Comment 3 Stefan Metzmacher 2011-01-06 07:38:41 UTC
Created attachment 6185 [details]
valgrind output
Comment 4 Stefan Metzmacher 2011-01-06 07:39:51 UTC
Notis this:

»·······# adding talloc_report_full(NULL, stdout);
»·······# add the end of
»·······# py_package_PrimaryKerberosString_set_string()
»·······# demonstrates that.
»·······#
»·······# you need this:
»·······# rm bin/default/librpc/gen_ndr/py_drsblobs*.o
»·······# rm bin/default/source4/librpc/python-dcerpc-drsblobs.so
Comment 5 Matthieu Patou 2012-10-05 07:36:52 UTC
jelmer any news ?
Comment 6 Jelmer Vernooij 2012-10-05 09:16:38 UTC
Sorry, missed this earlier. I'll have a look.
Comment 7 Jelmer Vernooij 2013-05-01 02:13:40 UTC
This is a genuine bug that we have known about for quite some time but I never spent enough time on it to fix it.

Metze, did you recompile with Py_USING_MEMORY_DEBUGGER uncommented before running valgrind? If not, the valgrind is probably a red herring, but the rest of the bug and script are still correct and very useful.
Comment 8 Petr Viktorin 2015-07-31 14:53:38 UTC
I'm pretty new to talloc, so this might be way off. Also I don't know Perl yet; I'm only looking at the generated code.

But, in bin/default/librpc/gen_ndr/py_drsblobs.c I see:

    static PyObject *py_package_PrimaryKerberosCtr3_get_salt(PyObject *obj, void *closure)
    {
        struct package_PrimaryKerberosCtr3 *object = (struct package_PrimaryKerberosCtr3 *)pytalloc_get_ptr(obj);
        PyObject *py_salt;
        py_salt = pytalloc_reference_ex(&package_PrimaryKerberosString_Type, pytalloc_get_mem_ctx(obj), &object->salt);
        return py_salt;
    }

which seems it wants to make a new reference to "object->salt" from "obj".


But, if you look into pytalloc_util.c, you'll see that
    pytalloc_reference_ex(PyTypeObject *py_type, TALLOC_CTX *mem_ctx, void *ptr)
it doesn't add such a reference.
It only makes a reference from the new object to "mem_ctx", assuming that "mem_ctx" already contains "ptr"; .

Hopefully that's helpful. I'm still somewhat confused by the code, but I wanted to post this before next week's vacation.


By the way, in the generated py_drsblobs.c's py_package_PrimaryKerberosCtr3_set_salt, any previous value of salt is never deallocated.
Comment 9 Andrew Bartlett 2017-01-03 04:24:50 UTC
Fixed in Samba 4.4 with 22905ceaf55f30bab996a583b23bc2eae08615da from master commit 9e07f3a13b41be1f019887581b2a2bd049039a3d 

The script now runs clean under valgrind
Comment 10 Andrew Bartlett 2019-04-01 21:20:50 UTC
Moving PIDL bugs into Samba as we do not release PIDL separately.