zephyrproject-rtos/west

Log message in west command __init__ func may cause infinite call stack

Opened this issue · 3 comments

If use any log api like self.err or self.die in WestCommand's __init__ function, it may throw following exception:

  ........................

  File "/home/yves/.local/lib/python3.10/site-packages/west/commands.py", line 287, in _get_config
    self.die(f"can't run west {self.name}; it requires config "
  File "/home/yves/.local/lib/python3.10/site-packages/west/commands.py", line 504, in die
    self.err(*args, fatal=True)
  File "/home/yves/.local/lib/python3.10/site-packages/west/commands.py", line 487, in err
    if self.color_ui:
  File "/home/yves/.local/lib/python3.10/site-packages/west/commands.py", line 510, in color_ui
    return self.config.getboolean('color.ui', default=True)
  File "/home/yves/.local/lib/python3.10/site-packages/west/commands.py", line 287, in _get_config
    self.die(f"can't run west {self.name}; it requires config "
  File "/home/yves/.local/lib/python3.10/site-packages/west/commands.py", line 504, in die
    self.err(*args, fatal=True)
  File "/home/yves/.local/lib/python3.10/site-packages/west/commands.py", line 487, in err
    if self.color_ui:
  File "/home/yves/.local/lib/python3.10/site-packages/west/commands.py", line 510, in color_ui
    return self.config.getboolean('color.ui', default=True)
  File "/home/yves/.local/lib/python3.10/site-packages/west/commands.py", line 287, in _get_config
    self.die(f"can't run west {self.name}; it requires config "
  File "/home/yves/.local/lib/python3.10/site-packages/west/commands.py", line 504, in die
    self.err(*args, fatal=True)
  File "/home/yves/.local/lib/python3.10/site-packages/west/commands.py", line 487, in err
    if self.color_ui:
RecursionError: maximum recursion depth exceeded

In __init__ phase, the variable config is None, but the property color_ui try infinitely get it from config

Thanks for the report! Can you please share:

  • The west commit used
  • The one-line diff
  • Just in case, the manifest used.

Hi, @marc-hb , sorry my description is not quite clear. The issue happens on a west extension command and my installed west version is v1.2.0

Hook following extension command to a west project can reproduce the issue.

# example.py
from west.commands import WestCommand
class Example(WestCommand):
    def __init__(self):
        super().__init__(
          'example',
          # Keep this in sync with the string in west-commands.yml.
          'example',
          "example")
        
        # This may raise an exception
        self.err("show a log")

    def do_add_parser(self, parser_adder):
        parser = self._parser(parser_adder, epilog='')
        return parser
    
    def do_run(self, args):
        pass

Initialization is often tricky: ordering dependencies can be hard. So it's not a major surprise that some things don't work (yet) in an __init__() method. I just had a look at initializations in zephyr/scripts/west_commands/ and none of them has any logic, just constants. This should probably be better documented but: "In Rome, do as the Romans do?" I recommend you defer any logic somewhere after __init__() time.

This being said, error-handling should be better: better message and nothing should ever trigger an infinite loop/recursion.