valgrind suppressions file for Python tests

[mmathesius]

Progress towards #333.

Valgrind failures for Python scripts are currently considered non-fatal if MMD_IGNORE_PYTHON_VALGRIND is set in the environment--which is currently done in modulemd/meson.build.

The big question is... what should be included in the suppression file? For starters, I included a few from the valgrind-python.supp file included in the Python 2 and 3 installs that were prevalent when running the tests.

Also, am I allowed to include contrib/valgrind/grindmerge here? It's a handy script I found for extracting and merging suppression entries from the test logfile.

[mmathesius]

FWIW, when including the following four very specific valgrind suppression entries (in addition to the three general entries already included), the valgrind test has no complaints. But I haven't been able to figure out if they're real problems or not.

{
   <insert_a_suppression_name_here>
   Memcheck:Leak
   match-leak-kinds: definite
   fun:malloc
   fun:g_malloc
   fun:g_slice_alloc
   fun:g_hash_table_new_full
   fun:modulemd_yaml_parse_string_set
   fun:modulemd_module_stream_v1_parse_licenses
   fun:modulemd_module_stream_v1_parse_yaml
   fun:modulemd_module_stream_read_yaml
   fun:modulemd_module_stream_read_string
   fun:ffi_call_unix64
   fun:ffi_call
   obj:/usr/lib64/python3.7/site-packages/gi/_gi.cpython-37m-x86_64-linux-gnu.so
}
{
   <insert_a_suppression_name_here>
   Memcheck:Leak
   match-leak-kinds: definite
   fun:malloc
   fun:_PyMem_RawWcsdup
   obj:/usr/lib64/libpython3.7m.so.1.0
   fun:_Py_InitializeCore
   obj:/usr/lib64/libpython3.7m.so.1.0
   obj:/usr/lib64/libpython3.7m.so.1.0
   fun:_Py_UnixMain
   fun:(below main)
}
{
   <insert_a_suppression_name_here>
   Memcheck:Cond
   fun:PyUnicode_Decode
   fun:PyUnicode_FromEncodedObject
   obj:/usr/lib64/libpython3.7m.so.1.0
   fun:_PyObject_FastCallKeywords
   obj:/usr/lib64/libpython3.7m.so.1.0
   fun:_PyEval_EvalFrameDefault
   fun:_PyFunction_FastCallDict
   fun:_PyObject_Call_Prepend
   obj:/usr/lib64/libpython3.7m.so.1.0
   fun:_PyObject_FastCallKeywords
   obj:/usr/lib64/libpython3.7m.so.1.0
   fun:_PyEval_EvalFrameDefault
}
{
   <insert_a_suppression_name_here>
   Memcheck:Leak
   match-leak-kinds: definite
   fun:malloc
   fun:g_malloc
   fun:g_strdup
   obj:/usr/lib64/python3.7/site-packages/gi/_gi.cpython-37m-x86_64-linux-gnu.so
   obj:/usr/lib64/python3.7/site-packages/gi/_gi.cpython-37m-x86_64-linux-gnu.so
   obj:/usr/lib64/python3.7/site-packages/gi/_gi.cpython-37m-x86_64-linux-gnu.so
   obj:/usr/lib64/python3.7/site-packages/gi/_gi.cpython-37m-x86_64-linux-gnu.so
   obj:/usr/lib64/python3.7/site-packages/gi/_gi.cpython-37m-x86_64-linux-gnu.so
   fun:PyObject_SetAttr
   fun:_PyEval_EvalFrameDefault
   fun:_PyFunction_FastCallKeywords
   obj:/usr/lib64/libpython3.7m.so.1.0
}

[sgallagher]

Progress towards #333.

Valgrind failures for Python scripts are currently considered non-fatal if MMD_IGNORE_PYTHON_VALGRIND is set in the environment--which is currently done in modulemd/meson.build.

The whole point of adding the python suppressions is so that we should be treating new failures as fatal. I'm not sure why this option is useful, much less enabled by default.

The big question is... what should be included in the suppression file? For starters, I included a few from the valgrind-python.supp file included in the Python 2 and 3 installs that were prevalent when running the tests.

Short answer: we need to filter out anything that is coming from one of our dependencies (python itself, pyGObject, etc.). I see in your other comment that you are unsure whether those four remaining suppressions would be correct; we need to investigate that further.

That said, I'd be satisfied with being overzealous and including them if only because it would still allow us to have a baseline to notice if anything new appeared.

Also, am I allowed to include contrib/valgrind/grindmerge here? It's a handy script I found for extracting and merging suppression entries from the test logfile.

The license header says nothing about permission to modify it (it only says "used and copied"), so I'm not sure whether including it in the upstream sources would be acceptable.

[sgallagher]

FWIW, when including the following four very specific valgrind suppression entries (in addition to the three general entries already included), the valgrind test has no complaints. But I haven't been able to figure out if they're real problems or not.

{
   <insert_a_suppression_name_here>
   Memcheck:Leak
   match-leak-kinds: definite
   fun:malloc
   fun:g_malloc
   fun:g_slice_alloc
   fun:g_hash_table_new_full
   fun:modulemd_yaml_parse_string_set
   fun:modulemd_module_stream_v1_parse_licenses
   fun:modulemd_module_stream_v1_parse_yaml
   fun:modulemd_module_stream_read_yaml
   fun:modulemd_module_stream_read_string
   fun:ffi_call_unix64
   fun:ffi_call
   obj:/usr/lib64/python3.7/site-packages/gi/_gi.cpython-37m-x86_64-linux-gnu.so
}

This one might be real (though it might be in pyGObject rather than libmodulemd). I'll take a look.

I'm comfortable with suppressing the other three, since the traceback involved doesn't include libmodulemd, pyGObject or FFI in any way, so it's almost certainly not our fault. As long as we aren't adding any new leaks going forward, that will still be a win.

[sgallagher]

@mmathesius Yup, that first one was indeed real. See #443

The other three are safe to suppress.

[sgallagher]

Please add the remaining three suppressions, drop the MMD_IGNORE_PYTHON_VALGRIND environment variable handling and drop grindmerge from the commit.

[sgallagher]

This looks good to me. Assuming it all passes CI, is there any reason not to merge it? (Aside from fixing the "WIP" in the summary)

[mmathesius]

@mmathesius Yup, that first one was indeed real. See #443

The other three are safe to suppress.

@sgallagher Thanks for the feedback, and for tracking that real one down!

We can expect CI to fail with my latest push until #443 is merged.