adafruit / circuitpython

CircuitPython - a Python implementation for teaching coding with microcontrollers
https://circuitpython.org
Other
4.11k stars 1.22k forks source link

master: RuntimeError: maximum recursion depth exceeded #1085

Closed notro closed 6 years ago

notro commented 6 years ago

I needed to rebase #1064 on yesterdays pin->number change and my tests started to fail:

pi@cp:~/work/circuitpython/cp-smbusslave/tests $ pytest --board=feather_m0_express --bus=1 smbus/
============================================================== test session starts ===============================================================
platform linux -- Python 3.5.3, pytest-3.6.3, py-1.5.4, pluggy-0.6.0
rootdir: /home/pi/work/circuitpython/cp-smbusslave/tests, inifile:
plugins: repeat-0.5.0, circuitpython-0.0.1
collected 170 items

smbus/test_ads1015_linux.py FFFFFFFFFFE                                                                                                    [  5%]
smbus/test_at24_linux.py ...........................................                                                                       [ 31%]
smbus/test_byte_word.py ................................................................                                                   [ 68%]
smbus/test_ds1307.py FE                                                                                                                    [ 69%]
smbus/test_ds1307_linux.py EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE                                                                       [ 92%]
smbus/test_mcp23008_linux.py EEEEEEEsssssE                                                                                                 [100%]

Exception:

Traceback (most recent call last):
  File "<stdin>", line 34, in <module>
  File "<stdin>", line 4, in ads1015slave_func
  File "/home/pi/circuitpython/i2cslave/disk/ads1015slave.py", line 2, in <module>
  File "/home/pi/circuitpython/i2cslave/disk/smbusslave.py", line 2, in <module>
  File "/home/pi/circuitpython/i2cslave/disk/smbusslave.py", line 6, in SMBusSlave
RuntimeError: maximum recursion depth exceeded

This seems to be the minimum to trigger the exception:

ads1015slave.py:

from smbusslave import SMBusSlave

class ADS1015Slave(SMBusSlave):
    pass

smbusslave.py:


class SMBusSlave:
    pass

Importing ads1015slave inside a function fails:

Adafruit CircuitPython 4.0.0-alpha-918-gdfa2581ff on 2018-08-03; Adafruit Feather M0 Express with samd21g18
>>>
paste mode; Ctrl-C to cancel, Ctrl-D to finish
=== def func():
===     import ads1015slave
===
=== func()
===
Traceback (most recent call last):
  File "<stdin>", line 4, in <module>
  File "<stdin>", line 2, in func
  File "ads1015slave.py", line 1, in <module>
  File "smbusslave.py", line 2, in <module>
RuntimeError: maximum recursion depth exceeded
>>>

Importing directly is fine:

Adafruit CircuitPython 4.0.0-alpha-918-gdfa2581ff on 2018-08-03; Adafruit Feather M0 Express with samd21g18
>>>
paste mode; Ctrl-C to cancel, Ctrl-D to finish
=== import ads1015slave
=== print(dir(ads1015slave))
===
['__file__', '__name__', 'SMBusSlave', 'ADS1015Slave']
>>>

Importing smbusslave inside a function is fine:

Adafruit CircuitPython 4.0.0-alpha-918-gdfa2581ff on 2018-08-03; Adafruit Feather M0 Express with samd21g18
>>>
paste mode; Ctrl-C to cancel, Ctrl-D to finish
=== def func():
===     import smbusslave
===     print(dir(smbusslave))
===
=== func()
===
['__file__', '__name__', 'SMBusSlave']
>>>

The tests passed when I was based on this commit:

commit 1fb81979b22a48c0933850f786e716a952d42c84 (tag: patchbase, tag: 4.0.0-alpha, origin/master, origin/HEAD, master)
Merge: 617351aeb bb28faf39
Author: arturo182 <arturo182@tlen.pl>
Date:   Wed Jul 18 09:03:56 2018 +0200

    Merge pull request #1036 from hathach/fix_nrf52840_serial

    Fix nrf52840 serial mentioned #1021
tannewt commented 6 years ago

The stack available is now dynamic and smaller than before by default. The default stack size likely needs to be tuned. The default is here: https://github.com/adafruit/circuitpython/blob/master/ports/atmel-samd/mpconfigport.h#L343

Please tune it using your tests and submit a PR. Thanks!

dhalbert commented 6 years ago

A long time ago I implemented instrumentation for measuring max stack usage in the atmel port. If you enable MICROPY_DEBUG_MODULES in ports/atmel-samd/ mpconfigport.h, it turns on uheap and ustack, and also turns on MICROPY_MAX_STACK_USAGE, which enables ustack.max_stack_usage().

THis is not in the nrf port, but it wouldn't be hard to add that.

Having said all that, I used the instrumentation when I added it, and came up with 12kB as a reasonable stack size. That includes a guard region of 1 or 2k (can't remember which).

I missed the reduction to 2kB in the latest dynamic build when I reviewed the PR. I think maybe we should bump it back up to 12kB for now, and do some more range checking when we have time.

notro commented 6 years ago

Thanks, this turned out to be real easy to tune. This is the bare minimum to make my tests pass:

import supervisor; supervisor.set_next_stack_limit(2048 + 256 + 64)
tannewt commented 6 years ago

@dhalbert That 2k doesn't include the 1k buffer or main's stack frame. The actual region is computed here: https://github.com/adafruit/circuitpython/blob/master/supervisor/shared/stack.c#L46

So, I'm happy to see it increased but I doubt we need it to be 12k.

jerryneedell commented 6 years ago

FYI -- I started seeing this issue repeatedly on a feather_m4_express with the current master. Increasing the default stack size from 2048 to 8192 on line 343 of mpconfigport.h fixed the problem

jerryneedell commented 6 years ago

I see the same setting in the nrf port, but it is not in the esp8266 -- is it not needed there?

notro commented 6 years ago

This is fixed now.