Residual reference management errors/memory leaks

[Gadgetoid]

I think this library still has a few gotchas remaining and I'm only barely qualified to find them. I've raised this issue for help digging them up.

For the first example; Running the following script shows at least one case where memory is leaked during exception handling:

import spidev
import time

bus = spidev.SpiDev(0, 0)

bus.mode = 0
bus.lsbfirst = False
bus.max_speed_hz = 80 * 1000000

bytes_total = 0
transfers_total = 0
last_update = time.time()

while True:
    try:
        bus.xfer([0b10101010] * 4097)
    except OverflowError:
        pass
    transfers_total += 1
    if time.time() - last_update >= 5.0:
        print(time.time(), transfers_total)
        last_update = time.time()

This gives the following output:

1590740245.1901975 31759
1590740250.1903563 64772
1590740257.996388 82785
1590740262.9964924 115641
1590740268.0020022 145691
1590740273.0742152 175049
1590740283.1994119 175748
Traceback (most recent call last):
  File "reftest.py", line 16, in <module>
    bus.xfer([0b10101010] * 4097)
MemoryError

In theory this has now created over 170,000 copies of our 4097 element list due to a missing Py_DECREF(seq) here:

py-spidev/spidev_module.c

Lines 487 to 491 in 3239755

	if (len > SPIDEV_MAXPATH) {
	snprintf(wrmsg_text, sizeof(wrmsg_text) - 1, wrmsg_listmax, SPIDEV_MAXPATH);
	PyErr_SetString(PyExc_OverflowError, wrmsg_text);
	return NULL;
	}

There may also be one missing here:

py-spidev/spidev_module.c

Lines 482 to 485 in 3239755

	if (!seq || len <= 0) {
	PyErr_SetString(PyExc_TypeError, wrmsg_list0);
	return NULL;
	}

But I'm not sure what the implications of conflating !seq and len(seq) <=0 might be. I would guess that !seq does not requre a Py_DECREF but that a valid seq that fails len(seq) <= 0 might. (Since a zero length list isn't going to use gobs of memory it might be difficult to invoke this even with synthetic tests)

Along the same thread, I think there is a Py_DECREF missing from here:

py-spidev/spidev_module.c

Lines 509 to 515 in 3239755

	} else {
	snprintf(wrmsg_text, sizeof(wrmsg_text) - 1, wrmsg_val, val);
	PyErr_SetString(PyExc_TypeError, wrmsg_text);
	free(xferptr);
	free(txbuf);
	free(rxbuf);
	return NULL;

This can be encouraged to fail with the following script:

import spidev
import time

bus = spidev.SpiDev(0, 0)

bus.mode = 0
bus.lsbfirst = False
bus.max_speed_hz = 80 * 1000000

bytes_total = 0
transfers_total = 0
last_update = time.time()

while True:
    try:
        bus.xfer(["1"] * 4096)
    except TypeError:
        pass
    transfers_total += 1
    if time.time() - last_update >= 5.0:
        print(time.time(), transfers_total)
        last_update = time.time()

which gives the output:

1590740834.8128057 31759
1590740839.816537 63912
1590740848.3375924 82785
1590740853.3499312 114935
1590740858.3557217 142895
1590740863.7203572 175049
1590740874.451649 175748
Traceback (most recent call last):
  File "reftest2.py", line 16, in <module>
    bus.xfer(["1"] * 4096)
MemoryError

I think anywhere there's a return NULL there's potentially a missing Py_DECREF(seq)

[Gadgetoid]

Ha! After trying to xfer 77 million empty list objects I hit a memory error. Sooner than I thought!

Test case:

import spidev
import time

bus = spidev.SpiDev(0, 0)

bus.mode = 0
bus.lsbfirst = False
bus.max_speed_hz = 80 * 1000000

bytes_total = 0
transfers_total = 0
last_update = time.time()

while True:
    try:
        bus.xfer([])
    except TypeError:
        pass
    transfers_total += 1
    if time.time() - last_update >= 5.0:
        print(time.time(), transfers_total)
        last_update = time.time()

result:

1590741402.4899502 74074125
1590741407.4900565 75578037
1590741412.4901624 77094181
Traceback (most recent call last):
  File "reftest2.py", line 16, in <module>
MemoryError

[plugwash]

But I'm not sure what the implications of conflating !seq and len(seq) <=0 might be.

Situations like that are what Py_XDECREF is for.