From 8b94506c3156a3b66faef7aac9a139f603772506 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micka=C3=ABl=20Schoentgen?= <contact@tiger-222.fr>
Date: Sun, 17 Feb 2019 09:13:26 +0100
Subject: [PATCH] Remove emitters which failed to start

---
 changelog.rst                   |  1 +
 src/watchdog/observers/api.py   |  8 ++++++--
 tests/test_emitter.py           |  2 +-
 tests/test_observer.py          | 27 +++++++++++++++++++++++++++
 tests/test_observers_polling.py |  7 ++++---
 tests/test_observers_winapi.py  | 12 ++++++------
 6 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/changelog.rst b/changelog.rst
index a738c1c4e..b71bd0e96 100644
--- a/changelog.rst
+++ b/changelog.rst
@@ -18,6 +18,7 @@ API changes
 - Fixed  a race condition crash when a directory is swapped for a file. 
 - Fixed the way we re-raise ``OSError``.
 - Fixed the path separator used in watchmedo. 
+- Remove emitters which failed to start.
 - We now generate sub created events only if ``recursive=True``.
 - Security fix in watchmedo: use ``yaml.safe_load()`` instead of ``yaml.load()``
 - Use ``scandir`` to save memory.
diff --git a/src/watchdog/observers/api.py b/src/watchdog/observers/api.py
index 01fc8a130..98d63a98c 100644
--- a/src/watchdog/observers/api.py
+++ b/src/watchdog/observers/api.py
@@ -248,8 +248,12 @@ def emitters(self):
         return self._emitters
 
     def start(self):
-        for emitter in self._emitters:
-            emitter.start()
+        for emitter in self._emitters.copy():
+            try:
+                emitter.start()
+            except Exception:
+                self._remove_emitter(emitter)
+                raise
         super(BaseObserver, self).start()
 
     def schedule(self, event_handler, path, recursive=False):
diff --git a/tests/test_emitter.py b/tests/test_emitter.py
index 3e414ccf7..792267d13 100644
--- a/tests/test_emitter.py
+++ b/tests/test_emitter.py
@@ -41,7 +41,7 @@
         InotifyFullEmitter,
     )
 elif platform.is_darwin():
-    pytestmark = pytest.mark.skip("FIXME: It is a matter of bad comparisons between bytes and str.")
+    pytestmark = pytest.mark.skip("FIXME: issue #546.")
     from watchdog.observers.fsevents2 import FSEventsEmitter as Emitter
 elif platform.is_windows():
     from watchdog.observers.read_directory_changes import (
diff --git a/tests/test_observer.py b/tests/test_observer.py
index 26229ccad..f40570ddf 100644
--- a/tests/test_observer.py
+++ b/tests/test_observer.py
@@ -115,3 +115,30 @@ def test_2_observers_on_the_same_path(observer, observer2):
 
     observer2.schedule(None, '')
     assert len(observer2.emitters) == 1
+
+
+def test_start_failure_should_not_prevent_further_try(monkeypatch, observer):
+    observer.schedule(None, '')
+    emitters = observer.emitters
+    assert len(emitters) == 1
+
+    # Make the emitter to fail on start()
+
+    def mocked_start():
+        raise OSError()
+
+    emitter = next(iter(emitters))
+    monkeypatch.setattr(emitter, "start", mocked_start)
+    with pytest.raises(OSError):
+        observer.start()
+    # The emitter should be removed from the list
+    assert len(observer.emitters) == 0
+
+    # Restoring the original behavior should work like there never be emitters
+    monkeypatch.undo()
+    observer.start()
+    assert len(observer.emitters) == 0
+
+    # Re-schduling the watch should work
+    observer.schedule(None, '')
+    assert len(observer.emitters) == 1
diff --git a/tests/test_observers_polling.py b/tests/test_observers_polling.py
index f94041a87..109163bb3 100644
--- a/tests/test_observers_polling.py
+++ b/tests/test_observers_polling.py
@@ -65,12 +65,14 @@ def event_queue():
 @pytest.fixture
 def emitter(event_queue):
     watch = ObservedWatch(temp_dir, True)
-    yield Emitter(event_queue, watch, timeout=0.2)
+    em = Emitter(event_queue, watch, timeout=0.2)
+    em.start()
+    yield em
+    em.stop()
 
 
 def test___init__(event_queue, emitter):
     SLEEP_TIME = 0.4
-    emitter.start()
 
     sleep(SLEEP_TIME)
     mkdir(p('project'))
@@ -165,7 +167,6 @@ def test___init__(event_queue, emitter):
 def test_delete_watched_dir(event_queue, emitter):
     SLEEP_TIME = 0.4
 
-    emitter.start()
     rm(p(''), recursive=True)
 
     sleep(SLEEP_TIME)
diff --git a/tests/test_observers_winapi.py b/tests/test_observers_winapi.py
index 52367e3db..4be47c5ed 100644
--- a/tests/test_observers_winapi.py
+++ b/tests/test_observers_winapi.py
@@ -59,14 +59,14 @@ def event_queue():
 @pytest.fixture
 def emitter(event_queue):
     watch = ObservedWatch(temp_dir, True)
-    yield WindowsApiEmitter(event_queue, watch, timeout=0.2)
+    em = WindowsApiEmitter(event_queue, watch, timeout=0.2)
+    yield em
+    em.stop()
 
 
-def test___init__():
+def test___init__(event_queue, emitter):
     SLEEP_TIME = 2
 
-    emitter.start()
-
     sleep(SLEEP_TIME)
     mkdir(p('fromdir'))
 
@@ -81,10 +81,10 @@ def test___init__():
     #   * unordered
     #   * non-unique
     # A multiset! Python's collections.Counter class seems appropriate.
-    expected = {[
+    expected = {
         DirCreatedEvent(p('fromdir')),
         DirMovedEvent(p('fromdir'), p('todir')),
-    ]}
+    }
 
     got = set()