pyrox.dev
1commit 4e68d9bfd12de51d8e6dc8ac622317f7755f1080
2Author: Palmer Cox <p@lmercox.com>
3Date: Tue May 14 23:07:48 2024 -0400
4
5 python312Packages.pybrowserid: Fix on Darwin
6
7 The WorkerPoolVerifier works by spawning one or more child processes in
8 order to run the verification process. This is fine on Linux when using
9 the default "fork" method of the multiprocessing module. However, this
10 breaks when using the "spawn" or "forkserver" method since those methods
11 require that the arguments passed into the Process object be pickleable.
12 Specifically we're passing threading.Lock objects to the child process
13 which are not pickleable.
14
15 Passing a Lock to a child process spawned via fork mostly does nothing -
16 the child process ends up with its own copy of the Lock which its free
17 to lock or unlock as it pleases and it has no effect on the parent
18 process. So, we don't actually need to pass the Lock to the child
19 process since it has never done anything. All we need to do is _not_
20 pass it since it causes errors when its passed because its not
21 pickleable. We don't need to re-create its functionality.
22
23 Anyway, there are two places where we are passing locks to the child
24 process. The first is the WorkerPoolVerifier class. This lock isn't
25 actually being used - its just passed because we're passing the "self"
26 object to the worker thread. So, we can fix this by making the target
27 method to run in the worker thread a free-function and only passing the
28 object we need - thus avoiding passing the unused Lock that triggers the
29 pickle error.
30
31 The other place that a Lock is passed ia via the FIFOCache. We can work
32 around this by making the Lock a global variable instead of an instance
33 variable. This shouldn't cause any significant performance issues
34 because the FIFOCache only holds this lock for brief periods when its
35 updating itself - its not doing any network call or long running
36 operations. Anyway, because its a global variable now, its not passed to
37 the process and we avoid the pickle error.
38
39 The other problem we have to fix are the tests on Darwin. There are two
40 problems - both due to Darwin defaulting to the "spawn" start method
41 for child Processes:
42
43 1. When we spawn a new Python process, it inherits the same __main__
44 module as its parent. When running tests, this __main__ module is the
45 unittest module. And, it start trying to run tests when its loaded.
46 This spawns more processes which also try to run tests, and so on.
47
48 2. The test code tries to mock out the network access. However, when we
49 spawn a new Python process these mocks are lost.
50
51 Anyway, we fix this issues by creating a temporary replacement for the
52 __main__ module which we install before running the WorkerPoolVerifier
53 tests. This module avoids trying to run the unit tests and also applies
54 the necessary mock to avoid network access.
55
56diff --git a/browserid/supportdoc.py b/browserid/supportdoc.py
57index d995fed..249b37e 100644
58--- a/browserid/supportdoc.py
59+++ b/browserid/supportdoc.py
60@@ -26,6 +26,9 @@ DEFAULT_TRUSTED_SECONDARIES = ("browserid.org", "diresworb.org",
61 "login.persona.org")
62
63
64+FIFO_CACHE_LOCK = threading.Lock()
65+
66+
67 class SupportDocumentManager(object):
68 """Manager for mapping hostnames to their BrowserID support documents.
69
70@@ -131,7 +134,6 @@ class FIFOCache(object):
71 self.max_size = max_size
72 self.items_map = {}
73 self.items_queue = collections.deque()
74- self._lock = threading.Lock()
75
76 def __getitem__(self, key):
77 """Lookup the given key in the cache.
78@@ -147,7 +149,7 @@ class FIFOCache(object):
79 # it hasn't been updated by another thread in the meantime.
80 # This is a little more work during eviction, but it means we
81 # can avoid locking in the common case of non-expired items.
82- self._lock.acquire()
83+ FIFO_CACHE_LOCK.acquire()
84 try:
85 if self.items_map[key][0] == timestamp:
86 # Just delete it from items_map. Trying to find
87@@ -157,7 +159,7 @@ class FIFOCache(object):
88 except KeyError:
89 pass
90 finally:
91- self._lock.release()
92+ FIFO_CACHE_LOCK.release()
93 raise KeyError
94 return value
95
96@@ -168,7 +170,7 @@ class FIFOCache(object):
97 there's enough room in the cache and evicting items if necessary.
98 """
99 now = time.time()
100- with self._lock:
101+ with FIFO_CACHE_LOCK:
102 # First we need to make sure there's enough room.
103 # This is a great opportunity to evict any expired items,
104 # helping to keep memory small for sparse caches.
105diff --git a/browserid/tests/dummy_main_module_for_unit_test_worker_processes.py b/browserid/tests/dummy_main_module_for_unit_test_worker_processes.py
106new file mode 100644
107index 0000000..388f86f
108--- /dev/null
109+++ b/browserid/tests/dummy_main_module_for_unit_test_worker_processes.py
110@@ -0,0 +1,29 @@
111+"""
112+This module is necessary as a hack to get tests to pass on Darwin.
113+
114+The reason is that the tests attempt to start up a multiprocessing.Process.
115+Doing that spawns a new Python process with the same __main__ module as the
116+parent process. The first problem is that the default start mode for Processes
117+on Darwin with "spawn" as opposed to "fork" as on Linux. This means that a
118+fully new Python interpretter is started with the same __main__ module. When
119+running tests, that __main__ module is the standard library unittest module.
120+Unfortunately, that module unconditionally runs unit tests without checking
121+that __name__ == "__main__". The end result is that every time a worker process
122+is spawned it immediately attemps to run the tests which spawn more processes
123+and so on.
124+
125+So, we work around that by replacing the __main__ module with this one before
126+any processes are spawned. This prevents the infinite spawning of processes
127+since this module doesn't try to run any unit tests.
128+
129+Additionally, this also patches the supportdoc fetching methods - which is
130+necessary to actually get the tests to pass.
131+"""
132+
133+import multiprocessing
134+
135+from browserid.tests.support import patched_supportdoc_fetching
136+
137+if multiprocessing.current_process().name != "MainProcess":
138+ patched_supportdoc_fetching().__enter__()
139+
140diff --git a/browserid/tests/test_verifiers.py b/browserid/tests/test_verifiers.py
141index d95ff8f..e8b867f 100644
142--- a/browserid/tests/test_verifiers.py
143+++ b/browserid/tests/test_verifiers.py
144@@ -454,15 +454,27 @@ class TestDummyVerifier(unittest.TestCase, VerifierTestCases):
145 class TestWorkerPoolVerifier(TestDummyVerifier):
146
147 def setUp(self):
148+ from browserid.tests import dummy_main_module_for_unit_test_worker_processes
149+ import sys
150+
151 super(TestWorkerPoolVerifier, self).setUp()
152+
153+ self.__old_main = sys.modules["__main__"]
154+ sys.modules["__main__"] = dummy_main_module_for_unit_test_worker_processes
155+
156 self.verifier = WorkerPoolVerifier(
157 verifier=LocalVerifier(["*"])
158 )
159
160 def tearDown(self):
161+ import sys
162+
163 super(TestWorkerPoolVerifier, self).tearDown()
164+
165 self.verifier.close()
166
167+ sys.modules["__main__"] = self.__old_main
168+
169
170 class TestShortcutFunction(unittest.TestCase):
171
172diff --git a/browserid/verifiers/workerpool.py b/browserid/verifiers/workerpool.py
173index c669107..d250222 100644
174--- a/browserid/verifiers/workerpool.py
175+++ b/browserid/verifiers/workerpool.py
176@@ -32,7 +32,6 @@ class WorkerPoolVerifier(object):
177 if verifier is None:
178 verifier = LocalVerifier()
179 self.num_procs = num_procs
180- self.verifier = verifier
181 # Create the various communication channels.
182 # Yes, this duplicates a lot of the logic from multprocessing.Pool.
183 # I don't want to have to constantly pickle the verifier object
184@@ -53,7 +52,7 @@ class WorkerPoolVerifier(object):
185 self._result_thread.start()
186 self._processes = []
187 for n in range(num_procs):
188- proc = multiprocessing.Process(target=self._run_worker)
189+ proc = multiprocessing.Process(target=_run_worker, args=(self._work_queue, verifier, self._result_queue))
190 self._processes.append(proc)
191 proc.start()
192
193@@ -128,21 +127,21 @@ class WorkerPoolVerifier(object):
194 self._waiting_conds[job_id] = (ok, result)
195 cond.notify()
196
197- def _run_worker(self):
198- """Method to run for the background worker processes.
199+def _run_worker(work_queue, verifier, result_queue):
200+ """Method to run for the background worker processes.
201
202- This method loops through jobs from the work queue, executing them
203- with the verifier object and pushing the result back via the result
204- queue.
205- """
206- while True:
207- job_id, args, kwds = self._work_queue.get()
208- if job_id is None:
209- break
210- try:
211- result = self.verifier.verify(*args, **kwds)
212- ok = True
213- except Exception as e:
214- result = e
215- ok = False
216- self._result_queue.put((job_id, ok, result))
217+ This method loops through jobs from the work queue, executing them
218+ with the verifier object and pushing the result back via the result
219+ queue.
220+ """
221+ while True:
222+ job_id, args, kwds = work_queue.get()
223+ if job_id is None:
224+ break
225+ try:
226+ result = verifier.verify(*args, **kwds)
227+ ok = True
228+ except Exception as e:
229+ result = e
230+ ok = False
231+ result_queue.put((job_id, ok, result))