Commits on May 7, 2020

1. fs: clean up Dir.read() uv_fs_t data before calling into JS …

   A call into JS can schedule another operation on the same `uv_dir_t`.
   In particular, when the handle is closed from the callback for a
   directory read operation, there previously was a race condition window:
   
   1. A `dir.read()` operation is submitted to libuv
   2. The read operation is finished by libuv, calling `AfterDirRead()`
   3. We call into JS
   4. JS calls dir.close()
   5. libuv posts the close request to a thread in the pool
   6. The close request runs, destroying the directory handle
   7. `AfterDirRead()` is being exited.
   
   Exiting the `FSReqAfterScope` in step 7 attempts to destroy the original
   uv_fs_t` from step 1, which now points to an `uv_dir_t` that has
   already been destroyed in step 5.
   
   By forcing the `FSReqAfterScope` to clean up before we call into JS,
   we can be sure that no other operations on the same `uv_dir_t` are
   submitted concurrently.
   
   This addresses issues observed when running with ASAN/valgrind.

   

   addaleax committed May 7, 2020
   Copy the full SHA

   fa868cb View commit details

   Browse the repository at this point in the history

2. fs: forbid concurrent operations on Dir handle …

   libuv does not expect concurrent operations on `uv_dir_t` instances,
   and will gladly create memory leaks, corrupt data, or crash the
   process.
   
   This patch forbids that, and:
   
   - Makes sure that concurrent async operations are run sequentially
   - Throws an exception if sync operations are attempted during an
     async operation
   
   The assumption here is that a thrown exception is preferable to
   a potential hard crash.
   
   This fully fixes flakiness from `parallel/test-fs-opendir` when
   run under ASAN.

   

   addaleax committed May 7, 2020
   Copy the full SHA

   e287d24 View commit details

   Browse the repository at this point in the history