waveform80 / picamera

A pure Python interface to the Raspberry Pi camera module
https://picamera.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
1.57k stars 355 forks source link

PiCamera and PiRGBAnalysis __exit__() methods cause ValueError: I/O operation on closed file. #671

Open juhaj opened 3 years ago

juhaj commented 3 years ago

The context __exit__ methods of PiCamera and PiRGBAnalysis try to close the output file twice when the latter is inside the scope of the former and an exception was raised inside the inner with. Reproduce with:

import picamera
import time
from picamera.array import PiRGBAnalysis

class MyColorAnalyzer(PiRGBAnalysis):
    def __init__(self, camera):
        super(MyColorAnalyzer, self).__init__(camera)

    def analyze(self, a):
        return

with picamera.PiCamera(resolution='160x90', framerate=24) as camera:
    start = time.perf_counter()
    recording = picamera.PiCameraCircularIO(
        camera,
        seconds=10,
        splitter_port=2
    )
    camera.start_recording(recording, 'rgb', splitter_port=2)
    with MyColorAnalyzer(camera) as analyzer:
        camera.start_recording(analyzer, 'rgb', splitter_port=1)
        while True and (time.perf_counter() - start < 3):
            camera.wait_recording(1)
        just("causing an exception here")

Workaround is to not use the context manager of either PiCamera or PiRGBAnalysis, with the obvious downsides.

juhaj commented 3 years ago

Forgot to mention: the culprit is the call to stream.flush() in the function stream_close in mmalobj.py, line 371:

def close_stream(stream, opened):
    """
    If *opened* is ``True``, then the ``close`` method of *stream* will be
    called. Otherwise, the function will attempt to call the ``flush`` method
    on *stream* (if one exists). This function essentially takes the output
    of :func:`open_stream` and finalizes the result.
    """
    if opened:
        stream.close()
    else:
        try:
            stream.flush()
        except AttributeError:
            pass

I don't know if it would be safe to just add ValueError to the except statement or whether it would be better to simply check we have already closed it. (But why have we?)