After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 772173 - cachestore: python fixes (FileNotFoundError: [Errno 2] No such file or directory …)
cachestore: python fixes (FileNotFoundError: [Errno 2] No such file or direct...
Status: RESOLVED FIXED
Product: gobject-introspection
Classification: Platform
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gobject-introspection Maintainer(s)
gobject-introspection Maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2016-09-29 11:42 UTC by Iain Lane
Modified: 2017-06-19 10:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
cachestore: Fix exception handling when removing files to work with python 2 and 3 (1.49 KB, patch)
2016-09-29 11:42 UTC, Iain Lane
none Details | Review
cachestore: Catch the right exception when unpickling (1.13 KB, patch)
2016-09-29 11:43 UTC, Iain Lane
none Details | Review
When handling errors according to errno, catch both IOError and OSError (4.44 KB, patch)
2017-01-27 12:16 UTC, Simon McVittie
committed Details | Review

Description Iain Lane 2016-09-29 11:42:54 UTC
1.50.0 is currently failing to build on ubuntu/powerpc with the following:

Traceback (most recent call last):
  • File "../../g-ir-doc-tool", line 66 in <module>
    sys.exit(doc_main(sys.argv))
  • File "../../giscanner/docmain.py", line 63 in doc_main
    transformer = Transformer.parse_from_gir(args.girfile, extra_include_dirs)
  • File "../../giscanner/transformer.py", line 209 in parse_from_gir
    self = cls(None)
  • File "../../giscanner/transformer.py", line 54 in __init__
    self._cachestore = CacheStore()
  • File "../../giscanner/cachestore.py", line 61 in __init__
    self._check_cache_version()
  • File "../../giscanner/cachestore.py", line 89 in _check_cache_version
    self._clean()
  • File "../../giscanner/cachestore.py", line 141 in _clean
    self._remove_filename(os.path.join(self._directory, filename))
  • File "../../giscanner/cachestore.py", line 123 in _remove_filename
    os.unlink(filename)
FileNotFoundError: [Errno 2] No such file or directory: '/home/laney/.cache/g-ir-scanner/c2d02fccbb2ef3ee15f8bd354ce74c94aedb088f'

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.
Comment 1 Iain Lane 2016-09-29 11:42:59 UTC
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.
Comment 2 Iain Lane 2016-09-29 11:43:04 UTC
Created attachment 336506 [details] [review]
cachestore: Catch the right exception when unpickling

cPickle's is called pickle.BadPickleGet, pickle's is called
pickle.UnpicklingError.
Comment 3 Simon McVittie 2017-01-27 12:16:00 UTC
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.
Comment 4 Iain Lane 2017-06-16 17:09:02 UTC
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.
Comment 5 Colin Walters 2017-06-16 21:47:33 UTC
Review of attachment 344406 [details] [review]:

Thanks!
Comment 6 Colin Walters 2017-06-16 21:47:34 UTC
Review of attachment 344406 [details] [review]:

Thanks!
Comment 7 Simon McVittie 2017-06-19 10:34:36 UTC
Comment on attachment 344406 [details] [review]
When handling errors according to errno, catch both IOError  and OSError

Pushed as 553bdc9f
Comment 8 Simon McVittie 2017-06-19 10:34:56 UTC
Fixed in git for 1.53.3, thanks for reviewing
Comment 9 Simon McVittie 2017-06-19 10:36:26 UTC
(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.
Comment 10 Iain Lane 2017-06-19 10:45:39 UTC
(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.