Commit Graph

4 Commits

Author SHA1 Message Date
Jun Wu
9dc21f8d0b codemod: import from the edenscm package
Summary:
D13853115 adds `edenscm/` to `sys.path` and code still uses `import mercurial`.
That has nasty problems if both `import mercurial` and
`import edenscm.mercurial` are used, because Python would think `mercurial.foo`
and `edenscm.mercurial.foo` are different modules so code like
`try: ... except mercurial.error.Foo: ...`, or `isinstance(x, mercurial.foo.Bar)`
would fail to handle the `edenscm.mercurial` version. There are also some
module-level states (ex. `extensions._extensions`) that would cause trouble if
they have multiple versions in a single process.

Change imports to use the `edenscm` so ideally the `mercurial` is no longer
imported at all. Add checks in extensions.py to catch unexpected extensions
importing modules from the old (wrong) locations when running tests.

Reviewed By: phillco

Differential Revision: D13868981

fbshipit-source-id: f4e2513766957fd81d85407994f7521a08e4de48
2019-01-29 17:25:32 -08:00
Kostia Balytskyi
825a9ddee5 windows: implement a more atomic rename/replace
Summary:
Our `rename` is not atomic:
```
def rename(src, dst):
    """Rename file src to dst, replacing dst if it exists"""
    try:
        os.rename(src, dst)
    except OSError as e:
        if e.errno != errno.EEXIST:
            raise
        unlink(dst)
        # What if the process is interrupted here?
        os.rename(src, dst)
```

However, the `MoveFileEx` Windows API provides a way to to
replace the existing file, thus eliminating the need to do `unlink`.

Unfortunately, it only works for files, not for dirs, therefore
we're introducing a new file-specific rename function.

Differential Revision: D12940555

fbshipit-source-id: a6749a9b16a285788de0f5c06d51a15c919166ce
2018-11-07 10:37:23 -08:00
Jun Wu
f1f4dc4c2c threading: add a way to workaround Python bug 29988
Summary:
The [bug 29988](https://bugs.python.org/issue29988) is, when Ctrl+C (or other
signal) interrupts a program between certain Python bytecode instructions,
`__exit__` might be skipped incorrectly.

A single signal can only skip a single `__exit__`. Therefore it can be
worked around by using a list of `with`s, like:

  with c, c, c, c, c:
      ....

Make `c.__enter__` and `__exit__` do the real `__enter__` and `__exit__` at
most once. To use the bug to skip all 5 `__exit__`s, 5 signals need to be
delivered at 5 tiny windows, which is unlikely to happen practically.  Besides,
we can increase the count of `c`s to become more confident.

Note this has to be implemented natively since pure Python code cannot
have a reliable way to record whether `__exit__` was called or not:

  self._obj.__exit__(...)
  # Insert SIGINT here
  self._exit_called = True

The inner object has to be native too to make `__exit__` atomic. Currently,
it's restricted to only the `Condition` object.

Reviewed By: markbt

Differential Revision: D10850510

fbshipit-source-id: 5c523a7bce568509641f8870d7ea381c0a99975c
2018-10-25 13:12:00 -07:00
Jun Wu
fe6f7ecf7b rust: reinvent Python's threading.Condition
Summary:
Python 2's `threading.Condition` and `threading.RLock` implementation are in
pure Python. Part of `RLock.acquire` looks like (simplified):

    def acquire(self, blocking=1):
        me = _get_ident()
        if self.__owner == me:
            self.__count = self.__count + 1
            return 1
        rc = self.__block.acquire(blocking)
        ########## Here #########
        if rc:
            self.__owner = me
            self.__count = 1
        return rc

If an interruption (ex. SIGTERM) happens at "HERE". The lock would be in an
inconsistent state. And if some `finally` block, or `__exit__` in a context
manager tries to release the lock, it could deadlock.

Similar problems also apply to `release`, `_acquire_restore`, and
`_release_save`. Basically, `self.__owner`, `self.__count` and `self.__block`
(i.e. the real lock) cannot be guaranteed in a consistent state in pure Python
world, because interruption can happen before any Python bytecode instruction
(but not inside a single Python bytecode instruction).

Therefore the interruption-safe implementation cannot be done in pure Python.
Use Rust to rescue.

The added test `streetest-condint.py` has a high chance to reproduce the
deadlock issue with Python 2.

Python 3 has a native RLock implementation, which makes things better. The
"Condition" implementation is not native and I haven't checked whether it
is sound or not.

Unfortunately, as part of testing, I hit https://bugs.python.org/issue29988 and
confirmed from the Rust world. That is, `__exit__` is not guarnateed called (!!)

That means native implementations still have a chance to be wrong, and there is
no easy way to fix it. `streetest-condint.py` was then updated to expose the
issue more easily.

The implementation is better than Python 2 stdlib, though.

Reviewed By: markbt

Differential Revision: D10517920

fbshipit-source-id: 394c9050c512ce2a0f9743c28ccfafe0f560141a
2018-10-25 13:12:00 -07:00