pycom / pycom-micropython-sigfox

A fork of MicroPython with the ESP32 port customized to run on Pycom's IoT multi-network modules.
MIT License
200 stars 166 forks source link

vfs_littlefs_file.c: Prevent double close of a file #524

Closed robert-hh closed 3 years ago

robert-hh commented 3 years ago

An attempt to close a file twice resulted in a crash, because buffers should have been freed again. This change checks first, if a file is already closed.

peter-pycom commented 3 years ago

https://github.com/pycom/pycom-micropython-sigfox/issues/523

geza-pycom commented 3 years ago

Hello! Good catch, it was hard to decode because without removing the _m_del_obj(pyb_file_objt, self) call the exception is dropped by _mp_load_methodmaybe(). I checked your example the issue happens and your fix solves it.

However with the following example, without your fix, the problem does not come alive: `

f = open("myfile.txt", "w") f.write("ssss") 4 f.close() f.close() Traceback (most recent call last): File "", line 1, in AttributeError: 'function' object has no attribute 'close' `

I think here the exception is correct, because the file object ("f") is deleted by _m_del_obj(pyb_file_objt, self).

It would be interesting to see why the crash happens with your example and why it does not with this other example.

robert-hh commented 3 years ago

I'm a little bit reluctant to delete python objects deep inside drivers, unless you know that they are not used any more. When I wrote that, I had a clear opinion why it was wrong here, but now I do not know it any more. Anyhow, it does not to keep it and let the garbage collector do the job later.

CLAassistant commented 3 years ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

geza-pycom commented 3 years ago

Thanks, this fix looks good !

robert-hh commented 3 years ago

Fixed in 1.20.3.b3