NilsRo / OctoPrint-SlicerEstimator

GNU Affero General Public License v3.0
5 stars 1 forks source link

Possible race condition with other plugins? #65

Closed paukstelis closed 1 year ago

paukstelis commented 1 year ago

I maintain the Power Failure plugin, and recently had a user describe an issue in which the recovery gcode file generated by the plugin was not being written. Disabling Slicer Estimator fixes this problem. It seems that perhaps there is some kind of race condition opening/closing the file stream that causes both plugins to fail. See log output below. Not sure if there is a simple or straightforward fix for this.

2023-07-16 10:01:32,934 - octoprint.plugins.powerfailure - INFO - Recovering from a print failure
2023-07-16 10:01:32,997 - octoprint.util.comm - INFO - Printer reports firmware name "Marlin 2.1.2 (Jan  4 2023 20:29:23)"
2023-07-16 10:01:33,039 - octoprint.util.comm - INFO - Firmware states that it supports temperature autoreporting
2023-07-16 10:01:33,093 - octoprint.util.comm - INFO - Firmware states that it supports emergency GCODEs to be sent without waiting for an acknowledgement first
2023-07-16 10:01:33,214 - octoprint.filemanager - ERROR - Error when calling preprocessor hook for plugin SlicerEstimator, ignoring
Traceback (most recent call last):
  File "/home/pi/oprint/lib/python3.7/site-packages/octoprint/filemanager/__init__.py", line 708, in add_file
    allow_overwrite=allow_overwrite,
  File "/home/pi/oprint/lib/python3.7/site-packages/octoprint/util/__init__.py", line 1686, in wrapper
    return f(*args, **kwargs)
  File "/home/pi/oprint/lib/python3.7/site-packages/octoprint_SlicerEstimator/__init__.py", line 181, in on_file_upload
    filedata = SlicerEstimatorFiledata(path, file_object, self)
  File "/home/pi/oprint/lib/python3.7/site-packages/octoprint_SlicerEstimator/metadata.py", line 120, in __init__
    self.slicer = SlicerEstimatorMetadataFiles.detect_slicer(self._file_object.path)
AttributeError: 'StreamWrapper' object has no attribute 'path'
2023-07-16 10:01:33,216 - octoprint.plugins.DisplayLayerProgress - INFO - FilePreProcessor. Checking LayerExpressions.
2023-07-16 10:01:33,216 - octoprint.plugins.DisplayLayerProgress - INFO - FilePreProcessor. LayerExpression valid. Start processing...
2023-07-16 10:01:33,432 - octoprint.plugin - ERROR - Error while calling plugin powerfailure
Traceback (most recent call last):
  File "/home/pi/oprint/lib/python3.7/site-packages/octoprint/plugin/__init__.py", line 275, in call_plugin
    result = getattr(plugin, method)(*args, **kwargs)
  File "/home/pi/oprint/lib/python3.7/site-packages/octoprint/util/__init__.py", line 1686, in wrapper
    return f(*args, **kwargs)
  File "/home/pi/oprint/lib/python3.7/site-packages/octoprint_powerfailure/__init__.py", line 296, in on_event
    self.check_recovery()
  File "/home/pi/oprint/lib/python3.7/site-packages/octoprint_powerfailure/__init__.py", line 130, in check_recovery
    recovery_fn = self.generateContinuation()
  File "/home/pi/oprint/lib/python3.7/site-packages/octoprint_powerfailure/__init__.py", line 211, in generateContinuation
    octoprint.filemanager.FileDestinations.LOCAL, recovery_fn, stream, allow_overwrite=True)
  File "/home/pi/oprint/lib/python3.7/site-packages/octoprint/filemanager/__init__.py", line 733, in add_file
    display=display,
  File "/home/pi/oprint/lib/python3.7/site-packages/octoprint/filemanager/storage.py", line 894, in add_file
    file_object.save(file_path)
  File "/home/pi/oprint/lib/python3.7/site-packages/octoprint/filemanager/util.py", line 102, in save
    shutil.copyfileobj(source, dest)
  File "/home/pi/oprint/lib/python3.7/shutil.py", line 79, in copyfileobj
    buf = fsrc.read(length)
  File "/home/pi/oprint/lib/python3.7/site-packages/octoprint/filemanager/util.py", line 211, in read
    line = self.input_stream.readline()
ValueError: readline of closed file
NilsRo commented 1 year ago

Hi.

It seems that the path property is missing. I do not know if it is optional or required from OctoPrint but it linked to the file uploaded before processing. I need that actually to determine the slicer for correct handling before parsing the GCODE.

Is it possible to simply trigger the recovery and debug it in your plugin? Also it is interesting what your plugin is doing. I can have a look then.

Cheers, Nils

paukstelis commented 1 year ago

I typically do programmatic testing using an OctoPrint instance running in a VM via the command-line. That way, I can just kill the server after a print is started. The recovery file is generated when the server is re-started and the virtual printer is connected.

NilsRo commented 1 year ago

Hi,

I implemented a check if path parameter is available and if not there is no slicer detection. Could you finally test if this is working in our case? You have to switch on development version in my plugin settings.

Finally I am not sure if the path parameter should be there as it has to be a type of DiskFileWrapper or if it is not required at this step. https://docs.octoprint.org/en/master/modules/filemanager.html#octoprint.filemanager.util.AbstractFileWrapper

But anyway it should not run into an exception anymore.

Cheers, Nils