From: JINMEI Tatuya <jinmei@isc.org>
Date: Mon, 31 Dec 2012 22:54:53 +0000 (-0800)
Subject: [711] catch and ignore permission error for unlinking log lock file.
X-Git-Tag: bind10-1.0.0-rc-release~99^2~22^2~1
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=0232cbe00f15efbe750b21c80973feb220bdc6c2;p=thirdparty%2Fkea.git

[711] catch and ignore permission error for unlinking log lock file.
---

diff --git a/src/bin/bind10/bind10_src.py.in b/src/bin/bind10/bind10_src.py.in
index 7440c3991a..d797bc4f2b 100755
--- a/src/bin/bind10/bind10_src.py.in
+++ b/src/bin/bind10/bind10_src.py.in
@@ -1206,7 +1206,19 @@ def remove_lock_files():
     for f in lockfiles:
         fname = lpath + '/' + f
         if os.path.isfile(fname):
-            os.unlink(fname)
+            try:
+                os.unlink(fname)
+            except OSError as e:
+                # We catch and ignore permission related error on unlink.
+                # This can happen if bind10 started with -u, created a lock
+                # file as a privileged user, but the directory is not writable
+                # for the changed user.  This setup will cause immediate
+                # start failure, and we leave verbose error message including
+                # the leftover lock file, so it should be acceptable to ignore
+                # it (note that it doesn't make sense to log this event at
+                # this poitn)
+                if e.errno != errno.EPERM and e.errno != errno.EACCES:
+                    raise
 
     return
 
diff --git a/src/bin/bind10/tests/bind10_test.py.in b/src/bin/bind10/tests/bind10_test.py.in
index e28bd1a41b..ccfa831f8c 100644
--- a/src/bin/bind10/tests/bind10_test.py.in
+++ b/src/bin/bind10/tests/bind10_test.py.in
@@ -2342,11 +2342,15 @@ class TestFunctions(unittest.TestCase):
         self.assertFalse(os.path.exists(self.lockfile_testpath))
         os.mkdir(self.lockfile_testpath)
         self.assertTrue(os.path.isdir(self.lockfile_testpath))
+        self.__isfile_orig = bind10_src.os.path.isfile
+        self.__unlink_orig = bind10_src.os.unlink
 
     def tearDown(self):
         os.rmdir(self.lockfile_testpath)
         self.assertFalse(os.path.isdir(self.lockfile_testpath))
         os.environ["B10_LOCKFILE_DIR_FROM_BUILD"] = "@abs_top_builddir@"
+        bind10_src.os.path.isfile = self.__isfile_orig
+        bind10_src.os.unlink = self.__unlink_orig
 
     def test_remove_lock_files(self):
         os.environ["B10_LOCKFILE_DIR_FROM_BUILD"] = self.lockfile_testpath
@@ -2370,6 +2374,28 @@ class TestFunctions(unittest.TestCase):
         # second call should not assert anyway
         bind10_src.remove_lock_files()
 
+    def test_remove_lock_files_fail(self):
+        # Permission error on unlink is ignored; other exceptions are really
+        # unexpected and propagated.
+        def __raising_unlink(unused, ex):
+            raise ex
+
+        bind10_src.os.path.isfile = lambda _: True
+        os_error = OSError()
+        bind10_src.os.unlink = lambda f: __raising_unlink(f, os_error)
+
+        os_error.errno = errno.EPERM
+        bind10_src.remove_lock_files() # no disruption
+
+        os_error.errno = errno.EACCES
+        bind10_src.remove_lock_files() # no disruption
+
+        os_error.errno = errno.ENOENT
+        self.assertRaises(OSError, bind10_src.remove_lock_files)
+
+        bind10_src.os.unlink = lambda f: __raising_unlink(f, Exception('bad'))
+        self.assertRaises(Exception, bind10_src.remove_lock_files)
+
     def test_get_signame(self):
         # just test with some samples
         signame = bind10_src.get_signame(signal.SIGTERM)