patchrmdir: new extension to workaround rmdir kernel issues

Summary:
We have encountered a kernel issue where `rmdir` a non-empty directory may race
with other things and hang in kernel for a long time.

This patch changes `os.rmdir` to avoid `rmdir` non-empty directories. It is
written in Cython calling the low-level `readdir` libc friends to make overhead
minimal.

Test Plan: Added a new test

Reviewers: #sourcecontrol, clm, rmcelroy

Reviewed By: rmcelroy

Subscribers: rmcelroy, simpkins, osandov, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4716711

Tasks: 16647532

Signature: t1:4716711:1489627923:7c7432748c1fd8c070ce257bd172feebd3807f65

.hgignore

@@ -11,6 +11,7 @@ syntax: regexp
 ^tests/report\.json
 .idea
 .testtimes*
+^hgext3rd/.*\.c$
 
 subinclude:cfastmanifest/.hgignore
 subinclude:linelog/.hgignore

hgext3rd/patchrmdir.pyx

@@ -0,0 +1,63 @@
+# patchrmdir.py
+#
+# Copyright 2017 Facebook, Inc.
+#
+# This software may be used and distributed according to the terms of the
+# GNU General Public License version 2 or any later version.
+
+"""patch rmdir
+
+Check if a directory is empty before trying to call rmdir on it. This works
+around some kernel issues.
+"""
+
+from mercurial import (
+    extensions,
+)
+import os
+import errno
+
+cdef extern from "dirent.h":
+    ctypedef struct DIR
+    struct dirent:
+        pass
+
+    DIR *opendir(const char *)
+    dirent *readdir(DIR *)
+    int closedir(DIR *)
+
+cdef int _countdir(const char *path):
+    """return min(3, the number of entries inside a directory).
+    return -1 if the directory cannot be opened.
+    """
+    cdef DIR *d = opendir(path)
+    if d == NULL:
+        return -1
+
+    cdef dirent *e
+    cdef int n = 0
+    while True:
+        e = readdir(d)
+        if e == NULL:
+            break
+        else:
+            n += 1
+            if n > 2:
+                break
+    closedir(d)
+    return n
+
+def _rmdir(orig, path):
+    n = _countdir(path)
+    if n >= 3:
+        # The number 3 is because most systems have "." and "..". For systems
+        # without them, we fallback to the original rmdir, the behavior should
+        # still be correct.
+        # Choose a slightly different error message other than "Directory not
+        # empty" so the test could notice the difference.
+        raise OSError(errno.ENOTEMPTY, 'Non-empty directory: %r' % path)
+    else:
+        return orig(path)
+
+def uisetup(ui):
+    extensions.wrapfunction(os, 'rmdir', _rmdir)

setup.py

@@ -234,6 +234,15 @@ else:
                 ],
             ),
         ],
+        'patchrmdir': [
+            Extension('hgext3rd.patchrmdir',
+                sources=['hgext3rd/patchrmdir.pyx'],
+                extra_compile_args=[
+                    '-std=c99',
+                    '-Wall', '-Wextra', '-Wconversion', '-pedantic',
+                ],
+            ),
+        ],
     }
 
 COMPONENTS = sorted(availablepackages + availableextmodules.keys() +
@@ -267,6 +276,7 @@ if os.name == 'nt':
 else:
     cythonmodules = [
         'linelog',
+        'patchrmdir',
     ]
 for cythonmodule in cythonmodules:
     if cythonmodule in components:

tests/test-patchrmdir.py

@@ -0,0 +1,65 @@
+from __future__ import absolute_import, print_function
+
+import os
+import sys
+
+# make it runnable directly without run-tests.py
+sys.path[0:0] = [os.path.join(os.path.dirname(__file__), '..')]
+
+if not sys.platform.startswith('linux'):
+    sys.stderr.write('skipped: linux required\n')
+    sys.exit(80)
+
+from hgext3rd import patchrmdir
+from mercurial import util
+
+patchrmdir.uisetup(None)
+
+testtmp = os.environ['TESTTMP']
+
+d1 = os.path.join(testtmp, 'd1')
+d2 = os.path.join(testtmp, 'd1', 'd2')
+
+os.mkdir(d1)
+os.mkdir(d2)
+
+def tryfunc(func):
+    try:
+        func()
+    except Exception as ex:
+        # normalize the error message across platforms
+        if 'Non-empty directory' not in str(ex):
+            ex = '*'
+        else:
+            ex = 'Non-empty directory'
+        print('  error: %s' % ex)
+    else:
+        print('  success')
+
+print('rmdir d1 - should fail with ENOTEMPTY')
+tryfunc(lambda: os.rmdir(d1))
+
+print('rmdir d1/d2 - should succeed')
+tryfunc(lambda: os.rmdir(d2))
+
+open(d2, 'w').close()
+
+print('rmdir d1 - should fail with ENOTEMPTY')
+tryfunc(lambda: os.rmdir(d1))
+
+os.unlink(d2)
+
+print('rmdir d1 - should succeed')
+tryfunc(lambda: os.rmdir(d1))
+
+print('rmdir d1 - should fail with ENOENT')
+tryfunc(lambda: os.rmdir(d1))
+
+os.mkdir(d1)
+os.mkdir(d2)
+
+print('removedirs d2 (and d1) - should succeed')
+tryfunc(lambda: util.removedirs(d2))
+
+print('removedirs d1 - should fail with ENOENT')
+tryfunc(lambda: util.removedirs(d1))

tests/test-patchrmdir.py.out

@@ -0,0 +1,14 @@
+rmdir d1 - should fail with ENOTEMPTY
+  error: Non-empty directory
+rmdir d1/d2 - should succeed
+  success
+rmdir d1 - should fail with ENOTEMPTY
+  error: Non-empty directory
+rmdir d1 - should succeed
+  success
+rmdir d1 - should fail with ENOENT
+  error: *
+removedirs d2 (and d1) - should succeed
+  success
+removedirs d1 - should fail with ENOENT
+  error: *