Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

trollius bug causes problems for ZEO testsuite in Python 2 environments #176

Closed
d-maurer opened this issue Apr 24, 2021 · 4 comments
Closed

Comments

@d-maurer
Copy link
Contributor

The py27-msgpack1 job of "https://github.com/zopefoundation/ZEO/pull/175/checks?check_run_id=2425420460" has ZODB.tests.testblob.ClientStorageNonSharedBlobsBlobUndoTests.testUndoAfterConsumption failed:

Error in test testUndoAfterConsumption (ZODB.tests.testblob.ClientStorageNonSharedBlobsBlobUndoTests)
Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/2.7.18/x64/lib/python2.7/unittest/case.py", line 329, in run
    testMethod()
  File "/home/runner/work/ZEO/ZEO/.tox/py27-msgpack1/lib/python2.7/site-packages/ZODB/tests/testblob.py", line 236, in testUndoAfterConsumption
    database.close()
  File "/home/runner/work/ZEO/ZEO/.tox/py27-msgpack1/lib/python2.7/site-packages/ZODB/DB.py", line 645, in close
    self._mvcc_storage.close()
  File "/home/runner/work/ZEO/ZEO/.tox/py27-msgpack1/lib/python2.7/site-packages/ZODB/mvccadapter.py", line 68, in close
    self._storage.close()
  File "/home/runner/work/ZEO/ZEO/.tox/py27-msgpack1/lib/python2.7/site-packages/ZEO/tests/testZEO.py", line 1739, in close
    ClientStorage.close(self)
  File "/home/runner/work/ZEO/ZEO/.tox/py27-msgpack1/lib/python2.7/site-packages/ZEO/ClientStorage.py", line 305, in close
    self._server.close()
  File "/home/runner/work/ZEO/ZEO/.tox/py27-msgpack1/lib/python2.7/site-packages/ZEO/asyncio/client.py", line 888, in close
    self.loop.call_soon_threadsafe(self.loop.stop)
  File "/home/runner/work/ZEO/ZEO/.tox/py27-msgpack1/lib/python2.7/site-packages/trollius/base_events.py", line 499, in call_soon_threadsafe
    self._write_to_self()
  File "/home/runner/work/ZEO/ZEO/.tox/py27-msgpack1/lib/python2.7/site-packages/trollius/selector_events.py", line 166, in _write_to_self
    wrap_error(csock.send, b'\0')
  File "/home/runner/work/ZEO/ZEO/.tox/py27-msgpack1/lib/python2.7/site-packages/trollius/py33_exceptions.py", line 127, in wrap_error
    return func(*args, **kw)
error: [Errno 9] Bad file descriptor

trollius expects a race condition in its _write_to_self and has a try ... except which should handle it. However, it wrongly uses OSError rather than socket.error (under Python 3, socket.error is a subclass of OSError but not under Python 2). Thus, this is a trollius bug.

Apart from testsuite problems, there are likely no serious consequences.

@d-maurer
Copy link
Contributor Author

@jamadden Can you fix trollius: a reliable test suite helps to identify serious problems.

@d-maurer d-maurer changed the title Potential race condition revealed by testUndoAfterConsumption trollius bug causes problems for ZEO testsuite in Python 2 environments Apr 24, 2021
@jamadden
Copy link
Member

jamadden commented Apr 28, 2021

I did produce a new trollius 2.2.1 release for this.

I very much do not like doing that: The repository has been archived, the docs all claim that maintenance has ceased, the test suite only runs a small portion of what it should (because of other missing dependencies), there is no CI setup, building windows wheels is hard, etc. And I don't really know the code base. This was an easy, limited change, though.

In future, ZEO could try to monkey-patch trollius when it runs into trouble with it. In this case, adding from trollius.py33_exceptions import _MAP_ERRNO; _MAP_ERRNO[errno.EBADF] = OSError would have solved the issue. (At first I didn't realize a monkey-patch could solve this problem so I didn't suggest it.)

@d-maurer
Copy link
Contributor Author

d-maurer commented Apr 28, 2021 via email

@jamadden
Copy link
Member

Looking at the code it seemed that csock was a normal socket (which raises socket.error as exception); in order for a mapping to work, csock must be wrapped by something trollius specific.

Exactly. That's the job of wrap_error:

  File "/.../python2.7/site-packages/trollius/selector_events.py", line 166, in _write_to_self
    wrap_error(csock.send, b'\0')
  File "/.../python2.7/site-packages/trollius/py33_exceptions.py", line 127, in wrap_error
    return func(*args, **kw)
if PY2:
    def wrap_error(func, *args, **kw):
        """
        Wrap socket.error, IOError, OSError, select.error to raise new specialized
        exceptions of Python 3.3 like InterruptedError (PEP 3151).
        """
        try:
            return func(*args, **kw)
        except (socket.error, IOError, OSError) as exc:
            if ssl is not None and isinstance(exc, ssl.SSLError):
                raise
            _wrap_error(exc, _MAP_ERRNO, exc.errno)
            raise
        except select.error as exc:
            if exc.args:
                _wrap_error(exc, _MAP_ERRNO, exc.args[0])
            raise
def _wrap_error(exc, mapping, key):
    if key not in mapping:
        return
    new_err_cls = mapping[key]
    new_err = new_err_cls(*exc.args)

    # raise a new exception with the original traceback
    if hasattr(exc, '__traceback__'):
        traceback = exc.__traceback__
    else:
        traceback = sys.exc_info()[2]
    reraise(new_err_cls, new_err, traceback)

The only thing missing was that EBADF should present as OSError in _MAP_ERRNO.

NexediGitlab pushed a commit to SlapOS/slapos that referenced this issue Aug 9, 2021
ZEO 5.2.3 fixes several client concurrency bugs that could result in
data corruption. See changelog in https://pypi.org/project/ZEO/5.2.3/
for details.

trollius 2.2.post1 -> 2.2.1 update fixes "bad file descriptor" problem for ZEO5.
See zopefoundation/ZEO#176 for details.

futures 3.2.0 -> 3.3.0 update brings in more efficient use of threads in
ThreadPoolExecutor. See https://github.com/agronholm/pythonfutures/blob/master/CHANGES.rst#330
for details.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants