From 244e12088dfdb456c48acfcece584f3d55ad9c72 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 22 Jan 2016 14:09:55 +0100 Subject: Use Py_uintptr_t for atomic pointers Issue #26161: Use Py_uintptr_t instead of void* for atomic pointers in pyatomic.h. Use atomic_uintptr_t when is used. Using void* causes compilation warnings depending on which implementation of atomic types is used. --- Include/pyatomic.h | 6 +++--- Python/ceval_gil.h | 8 ++++---- Python/pystate.c | 47 ++++++++++++++++++++++++----------------------- 3 files changed, 31 insertions(+), 30 deletions(-) diff --git a/Include/pyatomic.h b/Include/pyatomic.h index 892a217..89028ef 100644 --- a/Include/pyatomic.h +++ b/Include/pyatomic.h @@ -30,7 +30,7 @@ typedef enum _Py_memory_order { } _Py_memory_order; typedef struct _Py_atomic_address { - _Atomic void *_value; + atomic_uintptr_t _value; } _Py_atomic_address; typedef struct _Py_atomic_int { @@ -61,7 +61,7 @@ typedef enum _Py_memory_order { } _Py_memory_order; typedef struct _Py_atomic_address { - void *_value; + Py_uintptr_t _value; } _Py_atomic_address; typedef struct _Py_atomic_int { @@ -98,7 +98,7 @@ typedef enum _Py_memory_order { } _Py_memory_order; typedef struct _Py_atomic_address { - void *_value; + Py_uintptr_t _value; } _Py_atomic_address; typedef struct _Py_atomic_int { diff --git a/Python/ceval_gil.h b/Python/ceval_gil.h index aafcbc2..8d38ee9 100644 --- a/Python/ceval_gil.h +++ b/Python/ceval_gil.h @@ -111,7 +111,7 @@ static _Py_atomic_int gil_locked = {-1}; static unsigned long gil_switch_number = 0; /* Last PyThreadState holding / having held the GIL. This helps us know whether anyone else was scheduled after we dropped the GIL. */ -static _Py_atomic_address gil_last_holder = {NULL}; +static _Py_atomic_address gil_last_holder = {0}; /* This condition variable allows one or several threads to wait until the GIL is released. In addition, the mutex also protects the above @@ -142,7 +142,7 @@ static void create_gil(void) #ifdef FORCE_SWITCHING COND_INIT(switch_cond); #endif - _Py_atomic_store_relaxed(&gil_last_holder, NULL); + _Py_atomic_store_relaxed(&gil_last_holder, 0); _Py_ANNOTATE_RWLOCK_CREATE(&gil_locked); _Py_atomic_store_explicit(&gil_locked, 0, _Py_memory_order_release); } @@ -178,7 +178,7 @@ static void drop_gil(PyThreadState *tstate) /* Sub-interpreter support: threads might have been switched under our feet using PyThreadState_Swap(). Fix the GIL last holder variable so that our heuristics work. */ - _Py_atomic_store_relaxed(&gil_last_holder, tstate); + _Py_atomic_store_relaxed(&gil_last_holder, (Py_uintptr_t)tstate); } MUTEX_LOCK(gil_mutex); @@ -240,7 +240,7 @@ _ready: _Py_ANNOTATE_RWLOCK_ACQUIRED(&gil_locked, /*is_write=*/1); if (tstate != (PyThreadState*)_Py_atomic_load_relaxed(&gil_last_holder)) { - _Py_atomic_store_relaxed(&gil_last_holder, tstate); + _Py_atomic_store_relaxed(&gil_last_holder, (Py_uintptr_t)tstate); ++gil_switch_number; } diff --git a/Python/pystate.c b/Python/pystate.c index 83f15fd..6d1c6d0 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -3,11 +3,13 @@ #include "Python.h" -#ifndef Py_BUILD_CORE -/* ensure that PyThreadState_GET() is a macro, not an alias to - * PyThreadState_Get() */ -# error "pystate.c must be compiled with Py_BUILD_CORE defined" -#endif +#define GET_TSTATE() \ + ((PyThreadState*)_Py_atomic_load_relaxed(&_PyThreadState_Current)) +#define SET_TSTATE(value) \ + _Py_atomic_store_relaxed(&_PyThreadState_Current, (Py_uintptr_t)(value)) +#define GET_INTERP_STATE() \ + (GET_TSTATE()->interp) + /* -------------------------------------------------------------------------- CAUTION @@ -54,7 +56,7 @@ static PyInterpreterState *interp_head = NULL; /* Assuming the current thread holds the GIL, this is the PyThreadState for the current thread. */ -_Py_atomic_address _PyThreadState_Current = {NULL}; +_Py_atomic_address _PyThreadState_Current = {0}; PyThreadFrameGetter _PyThreadState_GetFrame = NULL; #ifdef WITH_THREAD @@ -260,7 +262,7 @@ PyObject* PyState_FindModule(struct PyModuleDef* module) { Py_ssize_t index = module->m_base.m_index; - PyInterpreterState *state = PyThreadState_GET()->interp; + PyInterpreterState *state = GET_INTERP_STATE(); PyObject *res; if (module->m_slots) { return NULL; @@ -284,7 +286,7 @@ _PyState_AddModule(PyObject* module, struct PyModuleDef* def) "PyState_AddModule called on module with slots"); return -1; } - state = PyThreadState_GET()->interp; + state = GET_INTERP_STATE(); if (!def) return -1; if (!state->modules_by_index) { @@ -304,7 +306,7 @@ int PyState_AddModule(PyObject* module, struct PyModuleDef* def) { Py_ssize_t index; - PyInterpreterState *state = PyThreadState_GET()->interp; + PyInterpreterState *state = GET_INTERP_STATE(); if (!def) { Py_FatalError("PyState_AddModule: Module Definition is NULL"); return -1; @@ -331,7 +333,7 @@ PyState_RemoveModule(struct PyModuleDef* def) "PyState_RemoveModule called on module with slots"); return -1; } - state = PyThreadState_GET()->interp; + state = GET_INTERP_STATE(); if (index == 0) { Py_FatalError("PyState_RemoveModule: Module index invalid."); return -1; @@ -351,7 +353,7 @@ PyState_RemoveModule(struct PyModuleDef* def) void _PyState_ClearModules(void) { - PyInterpreterState *state = PyThreadState_GET()->interp; + PyInterpreterState *state = GET_INTERP_STATE(); if (state->modules_by_index) { Py_ssize_t i; for (i = 0; i < PyList_GET_SIZE(state->modules_by_index); i++) { @@ -429,7 +431,7 @@ tstate_delete_common(PyThreadState *tstate) void PyThreadState_Delete(PyThreadState *tstate) { - if (tstate == PyThreadState_GET()) + if (tstate == GET_TSTATE()) Py_FatalError("PyThreadState_Delete: tstate is still current"); #ifdef WITH_THREAD if (autoInterpreterState && PyThread_get_key_value(autoTLSkey) == tstate) @@ -443,11 +445,11 @@ PyThreadState_Delete(PyThreadState *tstate) void PyThreadState_DeleteCurrent() { - PyThreadState *tstate = PyThreadState_GET(); + PyThreadState *tstate = GET_TSTATE(); if (tstate == NULL) Py_FatalError( "PyThreadState_DeleteCurrent: no current tstate"); - _Py_atomic_store_relaxed(&_PyThreadState_Current, NULL); + SET_TSTATE(NULL); if (autoInterpreterState && PyThread_get_key_value(autoTLSkey) == tstate) PyThread_delete_key_value(autoTLSkey); tstate_delete_common(tstate); @@ -496,14 +498,14 @@ _PyThreadState_DeleteExcept(PyThreadState *tstate) PyThreadState * _PyThreadState_UncheckedGet(void) { - return PyThreadState_GET(); + return GET_TSTATE(); } PyThreadState * PyThreadState_Get(void) { - PyThreadState *tstate = PyThreadState_GET(); + PyThreadState *tstate = GET_TSTATE(); if (tstate == NULL) Py_FatalError("PyThreadState_Get: no current thread"); @@ -514,9 +516,9 @@ PyThreadState_Get(void) PyThreadState * PyThreadState_Swap(PyThreadState *newts) { - PyThreadState *oldts = PyThreadState_GET(); + PyThreadState *oldts = GET_TSTATE(); - _Py_atomic_store_relaxed(&_PyThreadState_Current, newts); + SET_TSTATE(newts); /* It should not be possible for more than one thread state to be used for a thread. Check this the best we can in debug builds. @@ -545,7 +547,7 @@ PyThreadState_Swap(PyThreadState *newts) PyObject * PyThreadState_GetDict(void) { - PyThreadState *tstate = PyThreadState_GET(); + PyThreadState *tstate = GET_TSTATE(); if (tstate == NULL) return NULL; @@ -569,8 +571,7 @@ PyThreadState_GetDict(void) int PyThreadState_SetAsyncExc(long id, PyObject *exc) { - PyThreadState *tstate = PyThreadState_GET(); - PyInterpreterState *interp = tstate->interp; + PyInterpreterState *interp = GET_INTERP_STATE(); PyThreadState *p; /* Although the GIL is held, a few C API functions can be called @@ -691,7 +692,7 @@ PyThreadState_IsCurrent(PyThreadState *tstate) { /* Must be the tstate for this thread */ assert(PyGILState_GetThisThreadState()==tstate); - return tstate == PyThreadState_GET(); + return tstate == GET_TSTATE(); } /* Internal initialization/finalization functions called by @@ -783,7 +784,7 @@ PyGILState_GetThisThreadState(void) int PyGILState_Check(void) { - PyThreadState *tstate = PyThreadState_GET(); + PyThreadState *tstate = GET_TSTATE(); return tstate && (tstate == PyGILState_GetThisThreadState()); } -- cgit v0.12 From 540a81c720fa5886f3f43a479424873a28a91512 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 25 Mar 2016 09:51:14 +0100 Subject: Issue #21925: Fix test_warnings for release mode Use -Wd comment line option to log the ResourceWarning. --- Lib/test/test_warnings/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_warnings/__init__.py b/Lib/test/test_warnings/__init__.py index 633b2ac..fffadad 100644 --- a/Lib/test/test_warnings/__init__.py +++ b/Lib/test/test_warnings/__init__.py @@ -962,12 +962,12 @@ a=A() # don't import the warnings module # (_warnings will try to import it) code = "f = open(%a)" % __file__ - rc, out, err = assert_python_ok("-c", code) + rc, out, err = assert_python_ok("-Wd", "-c", code) self.assertTrue(err.startswith(expected), ascii(err)) # import the warnings module code = "import warnings; f = open(%a)" % __file__ - rc, out, err = assert_python_ok("-c", code) + rc, out, err = assert_python_ok("-Wd", "-c", code) self.assertTrue(err.startswith(expected), ascii(err)) -- cgit v0.12 From 8a208510104a0ac5a34240ae4329b881ffcee998 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 25 Mar 2016 09:29:50 +0100 Subject: Issue #25654: * multiprocessing: open file with closefd=False to avoid ResourceWarning * _test_multiprocessing: open file with O_EXCL to detect bugs in tests (if a previous test forgot to remove TESTFN) * test_sys_exit(): remove TESTFN after each loop iteration Initial patch written by Serhiy Storchaka. --- Doc/library/multiprocessing.rst | 2 +- Lib/multiprocessing/forkserver.py | 8 +------- Lib/multiprocessing/process.py | 7 +------ Lib/multiprocessing/util.py | 23 +++++++++++++++++++++++ Lib/test/_test_multiprocessing.py | 18 +++++++++++++----- 5 files changed, 39 insertions(+), 19 deletions(-) diff --git a/Doc/library/multiprocessing.rst b/Doc/library/multiprocessing.rst index 8209ae9..684a59f 100644 --- a/Doc/library/multiprocessing.rst +++ b/Doc/library/multiprocessing.rst @@ -2689,7 +2689,7 @@ Beware of replacing :data:`sys.stdin` with a "file like object" in issues with processes-in-processes. This has been changed to:: sys.stdin.close() - sys.stdin = open(os.devnull) + sys.stdin = open(os.open(os.devnull, os.O_RDONLY), closefd=False) Which solves the fundamental issue of processes colliding with each other resulting in a bad file descriptor error, but introduces a potential danger diff --git a/Lib/multiprocessing/forkserver.py b/Lib/multiprocessing/forkserver.py index b27cba5..ad01ede 100644 --- a/Lib/multiprocessing/forkserver.py +++ b/Lib/multiprocessing/forkserver.py @@ -147,13 +147,7 @@ def main(listener_fd, alive_r, preload, main_path=None, sys_path=None): except ImportError: pass - # close sys.stdin - if sys.stdin is not None: - try: - sys.stdin.close() - sys.stdin = open(os.devnull) - except (OSError, ValueError): - pass + util._close_stdin() # ignoring SIGCHLD means no need to reap zombie processes handler = signal.signal(signal.SIGCHLD, signal.SIG_IGN) diff --git a/Lib/multiprocessing/process.py b/Lib/multiprocessing/process.py index 68959bf..bca8b7a 100644 --- a/Lib/multiprocessing/process.py +++ b/Lib/multiprocessing/process.py @@ -234,12 +234,7 @@ class BaseProcess(object): context._force_start_method(self._start_method) _process_counter = itertools.count(1) _children = set() - if sys.stdin is not None: - try: - sys.stdin.close() - sys.stdin = open(os.devnull) - except (OSError, ValueError): - pass + util._close_stdin() old_process = _current_process _current_process = self try: diff --git a/Lib/multiprocessing/util.py b/Lib/multiprocessing/util.py index ea5443d..1a2c0db 100644 --- a/Lib/multiprocessing/util.py +++ b/Lib/multiprocessing/util.py @@ -9,6 +9,7 @@ import os import itertools +import sys import weakref import atexit import threading # we want threading to install it's @@ -356,6 +357,28 @@ def close_all_fds_except(fds): assert fds[-1] == MAXFD, 'fd too large' for i in range(len(fds) - 1): os.closerange(fds[i]+1, fds[i+1]) +# +# Close sys.stdin and replace stdin with os.devnull +# + +def _close_stdin(): + if sys.stdin is None: + return + + try: + sys.stdin.close() + except (OSError, ValueError): + pass + + try: + fd = os.open(os.devnull, os.O_RDONLY) + try: + sys.stdin = open(fd, closefd=False) + except: + os.close(fd) + raise + except (OSError, ValueError): + pass # # Start a program with only specified fds kept open diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index 1bbbd0b..11a6310 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -455,13 +455,15 @@ class _TestSubclassingProcess(BaseTestCase): @classmethod def _test_stderr_flush(cls, testfn): - sys.stderr = open(testfn, 'w') + fd = os.open(testfn, os.O_WRONLY | os.O_CREAT | os.O_EXCL) + sys.stderr = open(fd, 'w', closefd=False) 1/0 # MARKER @classmethod def _test_sys_exit(cls, reason, testfn): - sys.stderr = open(testfn, 'w') + fd = os.open(testfn, os.O_WRONLY | os.O_CREAT | os.O_EXCL) + sys.stderr = open(fd, 'w', closefd=False) sys.exit(reason) def test_sys_exit(self): @@ -472,15 +474,21 @@ class _TestSubclassingProcess(BaseTestCase): testfn = test.support.TESTFN self.addCleanup(test.support.unlink, testfn) - for reason, code in (([1, 2, 3], 1), ('ignore this', 1)): + for reason in ( + [1, 2, 3], + 'ignore this', + ): p = self.Process(target=self._test_sys_exit, args=(reason, testfn)) p.daemon = True p.start() p.join(5) - self.assertEqual(p.exitcode, code) + self.assertEqual(p.exitcode, 1) with open(testfn, 'r') as f: - self.assertEqual(f.read().rstrip(), str(reason)) + content = f.read() + self.assertEqual(content.rstrip(), str(reason)) + + os.unlink(testfn) for reason in (True, False, 8): p = self.Process(target=sys.exit, args=(reason,)) -- cgit v0.12