search

l00ma / OctoPrint-roomTemp

Octopi : Displays room temperature in navbar
GNU Affero General Public License v3.0
9 stars 15 forks source link

Plugin breaks OctoPrint's startup if no Raspberry Pi is detected #16

Open foosel opened 5 years ago

foosel commented 5 years ago

The problem lies with these lines:

https://github.com/l00ma/OctoPrint-roomTemp/blob/1a6f426b9154607dff88322a82c168f8b5f26399/octoprint_roomtemp/__init__.py#L43-L45

You are nuking the whole on_after_startup handler with the sys.exit(0) here. Never call sys.exit from plugin code, you might be nuking important threads or even the whole server.

This issue so far wasn't really visible due to nothing absolutely important for OctoPrint's core happening in on_after_startup and hence potentially after this plugin has nuked it (it probably interfered with quite a number of third party plugins though). Starting with 1.3.10 this will change however, and this plugin is now breaking the whole server due to that sys.exit(0).

Additionally that gets triggered on a Pi3 even though it definitely IS a Raspberry Pi:

2018-11-20 12:40:11,317 - octoprint.plugins.roomtemp - INFO - This is not a Raspberry Pi - Plugin halted
pi@octopi:~ $ cat /proc/cpuinfo
[...]
Hardware        : BCM2835
Revision        : a02082
[...]
pi@octopi:~ $ cat /proc/device-tree/model
Raspberry Pi 3 Model B Rev 1.2

I'm blacklisting this plugin until this issue is fixed so it doesn't continue to break OctoPrint installs out there, in subtle or potentially not so subtle ways. I've also added additional hardening to OctoPrint against issues like this one that will be part of 1.3.10rc3+.

foosel commented 5 years ago

Quick update: the fact that the RPi3 is not detected in this case here is caused by an old version of the plugin that lacks RPi3 matching and also doesn't prompt to update due to #17.

The sys.exit still must go.