Opened 7 years ago

Last modified 19 months ago

#19363 new defect

16 bytes lost with every call to psi(real number)

Reported by: rws Owned by:
Priority: major Milestone: sage-8.5
Component: memleak Keywords: symbolic, function
Cc: Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

sage: for i in range(1000):
....:     _ = psi(.5)

in sage valgrindresults in:

==27695== 16,000 bytes in 1,000 blocks are possibly lost in loss record 5,181 of 5,373
==27695==    at 0x4C290CD: malloc (vg_replace_malloc.c:296)
==27695==    by 0x12569F89: sage_malloc (memory.c:1070)
==27695==    by 0x12569F89: __pyx_f_4sage_3ext_6memory_sage_sig_malloc (memory.c:821)
==27695==    by 0x1779FD8F: mpfr_init2 (init2.c:55)
==27695==    by 0x19A63BEE: __pyx_pf_4sage_5rings_9real_mpfr_10RealNumber___init__ (real
_mpfr.c:11360)
==27695==    by 0x19A63BEE: __pyx_pw_4sage_5rings_9real_mpfr_10RealNumber_1__init__ (rea
l_mpfr.c:11279)
==27695==    by 0x4EEE0BB: wrap_init (typeobject.c:4748)
==27695==    by 0x4E89A22: PyObject_Call (abstract.c:2529)
==27695==    by 0x4F3A316: PyEval_CallObjectWithKeywords (ceval.c:3902)
==27695==    by 0x4EA9FC4: wrapperdescr_call (descrobject.c:343)
==27695==    by 0x19A50924: __Pyx_PyObject_Call (real_mpfr.c:42295)
==27695==    by 0x19A50924: __pyx_pf_4sage_5rings_9real_mpfr_11RealLiteral___init__ (rea
l_mpfr.c:33947)
==27695==    by 0x19A50924: __pyx_pw_4sage_5rings_9real_mpfr_11RealLiteral_1__init__ (re
al_mpfr.c:33880)
==27695==    by 0x4EF3A9E: type_call (typeobject.c:745)
==27695==    by 0x19A3B400: __Pyx_PyObject_Call.constprop.100 (real_mpfr.c:42295)
==27695==    by 0x19A5E467: __pyx_pf_4sage_5rings_9real_mpfr_18create_RealNumber (real_m
pfr.c:35167)
==27695==    by 0x19A5E467: __pyx_pw_4sage_5rings_9real_mpfr_19create_RealNumber (real_m
pfr.c:34516)
==27695==    by 0x4F3F1A8: call_function (ceval.c:4033)
==27695==    by 0x4F3F1A8: PyEval_EvalFrameEx (ceval.c:2679)
==27695==    by 0x4F409CC: PyEval_EvalCodeEx (ceval.c:3265)
==27695==    by 0x4F40B01: PyEval_EvalCode (ceval.c:667)
==27695==    by 0x4F3F6AD: exec_statement (ceval.c:4730)
==27695==    by 0x4F3F6AD: PyEval_EvalFrameEx (ceval.c:1881)
==27695==    by 0x4F409CC: PyEval_EvalCodeEx (ceval.c:3265)
==27695==    by 0x4F3ECA0: fast_function (ceval.c:4129)
==27695==    by 0x4F3ECA0: call_function (ceval.c:4054)
==27695==    by 0x4F3ECA0: PyEval_EvalFrameEx (ceval.c:2679)
==27695==    by 0x4F409CC: PyEval_EvalCodeEx (ceval.c:3265)
==27695==    by 0x4F3ECA0: fast_function (ceval.c:4129)
==27695==    by 0x4F3ECA0: call_function (ceval.c:4054)
==27695==    by 0x4F3ECA0: PyEval_EvalFrameEx (ceval.c:2679)
==27695==    by 0x4F409CC: PyEval_EvalCodeEx (ceval.c:3265)
==27695==    by 0x4F3ECA0: fast_function (ceval.c:4129)
==27695==    by 0x4F3ECA0: call_function (ceval.c:4054)
==27695==    by 0x4F3ECA0: PyEval_EvalFrameEx (ceval.c:2679)
==27695==    by 0x4F409CC: PyEval_EvalCodeEx (ceval.c:3265)
==27695==    by 0x4F3ECA0: fast_function (ceval.c:4129)
==27695==    by 0x4F3ECA0: call_function (ceval.c:4054)
==27695==    by 0x4F3ECA0: PyEval_EvalFrameEx (ceval.c:2679)
==27695==    by 0x4F409CC: PyEval_EvalCodeEx (ceval.c:3265)

Change History (24)

comment:1 Changed 3 years ago by dimpase

  • Milestone changed from sage-6.9 to sage-8.5
  • Priority changed from minor to major

Indeed, still in 8.4.beta6 I see

sage: from sage.matroids.advanced import *
....: import gc
....: 
....: i = 0
....: while True:
....:     if i%10000==0:
....:         gc.collect()
....:         print get_memory_usage()
....:     i += 1
....:     _ = psi(.5)
....:     
7
2692.96875
0
2696.46875
0
2699.96875
0
2703.71875
0
2707.7265625
0
2711.734375
0
2715.87109375
0
2719.87890625
0
2723.88671875
0
2728.0234375
...

comment:2 Changed 3 years ago by dimpase

beta(-1.0,-0.5) leaks just as well, while gamma(-0.5) does not.

comment:3 Changed 3 years ago by jmantysalo

A step forward, this leaks too:

import gc
from sage.functions.gamma import Function_psi1
f = Function_psi1()

i = 0
while True:
    if i%1000 == 0:
        _ = gc.collect()
        print get_memory_usage()
    i += 1
    _ = f._eval_mpmath_(0.5)

comment:4 follow-up: Changed 3 years ago by dimpase

Does ._eval_mpmath_ matter here? It seems that _ = f(0.5) leaks just the same, no?

comment:5 in reply to: ↑ 4 Changed 3 years ago by jmantysalo

Replying to dimpase:

Does ._eval_mpmath_ matter here? It seems that _ = f(0.5) leaks just the same, no?

True, but I was trying to follow the code to see what function leaks. I did not found the original source.

comment:6 Changed 3 years ago by dimpase

The source is in pynac package, in C++. Let me dig this up more precisely. By the way, it's essential that the argument is a float, if it is an integer (e.g. 42) or a half-integer (e.g. 39/2) then there is no leak.

comment:7 follow-up: Changed 3 years ago by dimpase

Probably it's here, but I'm not sure https://github.com/pynac/pynac/blob/master/ginac/inifcns_gamma.cpp#L423

It does not seem to be easy to get more details, one has to know how pynac/ginac works, at least a bit... The cython interface is in src/sage/libs/pynac, and there are some py_phi*-functions defined there, but they don't seem to leak.

comment:8 follow-up: Changed 3 years ago by jmantysalo

Seems promising. But there seems to be no new or malloc in that file.

Personally I do not even understand what really happens when you say psi(0.5). Classes are created, used, discarded?

comment:9 Changed 3 years ago by rws

A first guess would be buggy usage of the Python C API in https://github.com/pynac/pynac/blob/master/ginac/numeric.cpp. Note that gamma(SR(-.5)) leaks while gamma(-.5) does not. The first calls Pynac (inifcns_gamma.cpp:gamma_eval), explanations can be found in https://github.com/pynac/pynac/wiki/%7C-floating-point-evaluation, while the second goes through src/sage/symbolic/function.pyx, calling the gamma member of RealNumber. Because mpfr has no psi nor beta, Pynac provides these by calling arb via the Python C API.

comment:10 in reply to: ↑ 8 Changed 3 years ago by rws

Replying to jmantysalo:

Personally I do not even understand what really happens when you say psi(0.5). Classes are created, used, discarded?

Please see https://github.com/pynac/pynac/wiki/%7C-functions

comment:11 follow-up: Changed 3 years ago by jdemeyer

Where is the code for converting a ball to a numeric?

comment:12 in reply to: ↑ 11 ; follow-up: Changed 3 years ago by rws

Replying to jdemeyer:

Where is the code for converting a ball to a numeric?

Balls are not handled by Pynac, Pynac uses the PyObjects returned by CBF() e.g. in CallBallMethod0Arg in numeric.cpp. Any returned PyObject result can be wrapped in a numeric, see e.g. the last two lines of numeric::lgamma.

comment:13 in reply to: ↑ 12 ; follow-up: Changed 3 years ago by jdemeyer

Replying to rws:

Any returned PyObject result can be wrapped in a numeric

Yes, and my question was: where is the code for that?

comment:14 in reply to: ↑ 7 Changed 3 years ago by rws

Replying to dimpase:

It does not seem to be easy to get more details, one has to know how pynac/ginac works, at least a bit...

See also #24398 for more documentation.

comment:15 in reply to: ↑ 13 Changed 3 years ago by rws

Replying to jdemeyer:

Replying to rws:

Any returned PyObject result can be wrapped in a numeric

Yes, and my question was: where is the code for that?

...see e.g. the last two lines of numeric::lgamma. Or do you mean how they are wrapped? Please see numeric.h for the class which has:

union Value {
        Value() {}
        Value(signed long int i) : _long(i) {}
        signed long int _long;
        mpz_t _bigint;
        mpq_t _bigrat;
        PyObject* _pyobject;
};

The line numeric rnum(ret); calls the constructor numeric::numeric and the line ex_to<numeric>(rnum.evalf(0, parent)); converts the ball it to the needed parent.

comment:16 Changed 3 years ago by jdemeyer

I had a quick look at the code. The only thing I see that could potentially leak is when a Python exception happens, for example in

    PyObject* ret_ball = PyObject_CallMethodObjArgs(aball, name, NULL);
    if (ret_ball == nullptr)
        throw(std::runtime_error("GiNaC::CallBallMethod1Arg(): PyObject_CallMethodObjArgs unsuccessful"));

you are throwing but not calling Py_DECREF().

comment:17 follow-up: Changed 3 years ago by dimpase

I probably am missing something, but how do you call Py_DECREF() on something that is NULL?

comment:18 in reply to: ↑ 17 Changed 3 years ago by jdemeyer

Replying to dimpase:

I probably am missing something, but how do you call Py_DECREF() on something that is NULL?

I meant calling Py_DECREF() on things which have been allocated before. Pseudo-code:

a = PySomething()
if a is NULL:
    throw("something failed")

b = PySomethingElse()
if b is NULL:
    # Should do Py_DECREF(a) here
    throw("something else failed")

comment:19 Changed 3 years ago by dimpase

Right, and there seems to be plenty of these around this very line you quoted

https://github.com/pynac/pynac/blob/798a8b7344bc688e27fc8e61cfa38076d912311f/ginac/numeric.cpp#L348

throws seem to be used there for error processing, so that's OK --- as long as you don't hit a runtime error, which ends up with a lot of uncleared Python objects - I hope I understand this right.

comment:20 Changed 3 years ago by jdemeyer

Yes. I just don't know if there are any scenarios where this error would be caught and execution continues as usual.

comment:21 follow-up: Changed 3 years ago by dimpase

By the way, doesn't iobj here

PyObject *iobj = Integer(res);

need a Py_DECREF()?

comment:22 in reply to: ↑ 21 Changed 3 years ago by rws

Replying to dimpase:

By the way, doesn't iobj here

PyObject *iobj = Integer(res);

need a Py_DECREF()?

The following PyTuple_SetItem steals the reference.

comment:23 Changed 3 years ago by dimpase

How does one use Logging_refctr in pynac? Is there something more than building with -DLogging_refctr in CXXFLAGS and then trying to make sense of the massive amount of output?

comment:24 Changed 19 months ago by mmezzarobba

Possibly related: #27536?

Note: See TracTickets for help on using tickets.