GNOME Bugzilla – Bug 772173
cachestore: python fixes (FileNotFoundError: [Errno 2] No such file or directory …)
Last modified: 2017-06-19 10:45:39 UTC
1.50.0 is currently failing to build on ubuntu/powerpc with the following: Traceback (most recent call last):
+ Trace 236710
sys.exit(doc_main(sys.argv))
transformer = Transformer.parse_from_gir(args.girfile, extra_include_dirs)
self = cls(None)
self._cachestore = CacheStore()
self._check_cache_version()
self._clean()
self._remove_filename(os.path.join(self._directory, filename))
os.unlink(filename)
Looking at the code shows that this is supposed to be swallowed but it's not due to a difference in Python 2 and 3 exception classes. After fixing that I found a cascading failure when unpickling. See the two attached patches.
Created attachment 336505 [details] [review] cachestore: Fix exception handling when removing files to work with python 2 and 3 Under Python 3, ENOENT results in a FileNotFoundException. In some versions of Python this is an OSError and in some versions it's an IOError, because OSError was folded into IOError. To keep support for Python 2, and support the new scheme in Python 3, catch both IOError and OSError in the same block and look for either accepted value in errno.
Created attachment 336506 [details] [review] cachestore: Catch the right exception when unpickling cPickle's is called pickle.BadPickleGet, pickle's is called pickle.UnpicklingError.
Created attachment 344406 [details] [review] When handling errors according to errno, catch both IOError and OSError Different Python versions are not completely consistent about the error that is raised and its class hierarchy: Python 3.5.3rc1 (default, Jan 3 2017, 04:40:57) >>> try: open('/foo') ... except Exception as e: print(e.__class__.__mro__) (<class 'FileNotFoundError'>, <class 'OSError'>, <class 'Exception'>, <class 'BaseException'>, <class 'object'>) Python 2.7.13 (default, Dec 18 2016, 20:19:42) >>> try: open('/foo') ... except Exception as e: print e.__class__.__mro (<type 'exceptions.IOError'>, <type 'exceptions.EnvironmentError'>, <type 'exceptions.StandardError'>, <type 'exceptions.Exception'>, <type 'exceptions.BaseException'>, <type 'object'>) This can lead to a race condition during cache cleaning, where two processes both try to delete the same file, and the one that loses the race fails. --- Sorry for the duplicate patch, I wrote it before searching Bugzilla. This one might be better than Attachment #336505 [details], because it fixes all instances of catching an error and looking at its errno.
Review of attachment 344406 [details] [review]: I was just reminded of this bug. Your patch looks good to me - it catches more places than mine and fixes the failure I was seeing. I'm not a maintainer so I won't set a-c_n, but if a review isn't forthcoming I think you could go ahead with pushing this.
Review of attachment 344406 [details] [review]: Thanks!
Comment on attachment 344406 [details] [review] When handling errors according to errno, catch both IOError and OSError Pushed as 553bdc9f
Fixed in git for 1.53.3, thanks for reviewing
(In reply to Iain Lane from comment #2) > Created attachment 336506 [details] [review] > cachestore: Catch the right exception when unpickling > > cPickle's is called pickle.BadPickleGet, pickle's is called > pickle.UnpicklingError. Is this patch still desired? If it is, please reopen. My patch did not address this case, only the OSError/IOError issues.
(In reply to Simon McVittie from comment #9) > (In reply to Iain Lane from comment #2) > > Created attachment 336506 [details] [review] [review] > > cachestore: Catch the right exception when unpickling > > > > cPickle's is called pickle.BadPickleGet, pickle's is called > > pickle.UnpicklingError. > > Is this patch still desired? If it is, please reopen. My patch did not > address this case, only the OSError/IOError issues. I don't think so, or at least I don't see a failure any more.