Mailman 3 November 2017 - Python-checkins

bpo-20891: Fix PyGILState_Ensure() (#4650) (#4655)
by Victor Stinner 30 Nov '17

30 Nov '17

https://github.com/python/cpython/commit/e10c9de9d74fd4c26b32e6719d96f04a5b… commit: e10c9de9d74fd4c26b32e6719d96f04a5be6987d branch: 3.6 author: Victor Stinner <victor.stinner(a)gmail.com> committer: GitHub <noreply(a)github.com> date: 2017-11-30T23:36:49+01:00 summary: bpo-20891: Fix PyGILState_Ensure() (#4650) (#4655) When PyGILState_Ensure() is called in a non-Python thread before PyEval_InitThreads(), only call PyEval_InitThreads() after calling PyThreadState_New() to fix a crash. Add an unit test in test_embed. Enhance also embedded tests, backport from master: * Add test_pre_initialization_api() * Set PYTHONIOENCODING environment variable in test_forced_io_encoding() (cherry picked from commit b4d1e1f7c1af6ae33f0e371576c8bcafedb099db) files: A Misc/NEWS.d/next/C API/2017-11-30-18-13-45.bpo-20891.wBnMdF.rst M Lib/test/test_capi.py M Programs/_testembed.c M Python/pystate.c diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index 3a29b69f957..6e4286ed881 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -401,23 +401,30 @@ def setUp(self): def tearDown(self): os.chdir(self.oldcwd) - def run_embedded_interpreter(self, *args): + def run_embedded_interpreter(self, *args, env=None): """Runs a test in the embedded interpreter""" cmd = [self.test_exe] cmd.extend(args) + if env is not None and sys.platform == 'win32': + # Windows requires at least the SYSTEMROOT environment variable to + # start Python. + env = env.copy() + env['SYSTEMROOT'] = os.environ['SYSTEMROOT'] + p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, - universal_newlines=True) + universal_newlines=True, + env=env) (out, err) = p.communicate() self.assertEqual(p.returncode, 0, "bad returncode %d, stderr is %r" % (p.returncode, err)) return out, err - def test_subinterps(self): + def test_repeated_init_and_subinterpreters(self): # This is just a "don't crash" test - out, err = self.run_embedded_interpreter() + out, err = self.run_embedded_interpreter('repeated_init_and_subinterpreters') if support.verbose: print() print(out) @@ -435,13 +442,14 @@ def _get_default_pipe_encoding(): def test_forced_io_encoding(self): # Checks forced configuration of embedded interpreter IO streams - out, err = self.run_embedded_interpreter("forced_io_encoding") + env = dict(os.environ, PYTHONIOENCODING="utf-8:surrogateescape") + out, err = self.run_embedded_interpreter("forced_io_encoding", env=env) if support.verbose: print() print(out) print(err) - expected_errors = sys.__stdout__.errors - expected_stdin_encoding = sys.__stdin__.encoding + expected_stream_encoding = "utf-8" + expected_errors = "surrogateescape" expected_pipe_encoding = self._get_default_pipe_encoding() expected_output = '\n'.join([ "--- Use defaults ---", @@ -469,13 +477,33 @@ def test_forced_io_encoding(self): "stdout: latin-1:replace", "stderr: latin-1:backslashreplace"]) expected_output = expected_output.format( - in_encoding=expected_stdin_encoding, - out_encoding=expected_pipe_encoding, + in_encoding=expected_stream_encoding, + out_encoding=expected_stream_encoding, errors=expected_errors) # This is useful if we ever trip over odd platform behaviour self.maxDiff = None self.assertEqual(out.strip(), expected_output) + def test_pre_initialization_api(self): + """ + Checks the few parts of the C-API that work before the runtine + is initialized (via Py_Initialize()). + """ + env = dict(os.environ, PYTHONPATH=os.pathsep.join(sys.path)) + out, err = self.run_embedded_interpreter("pre_initialization_api", env=env) + self.assertEqual(out, '') + self.assertEqual(err, '') + + def test_bpo20891(self): + """ + bpo-20891: Calling PyGILState_Ensure in a non-Python thread before + calling PyEval_InitThreads() must not crash. PyGILState_Ensure() must + call PyEval_InitThreads() for us in this case. + """ + out, err = self.run_embedded_interpreter("bpo20891") + self.assertEqual(out, '') + self.assertEqual(err, '') + class SkipitemTest(unittest.TestCase): diff --git a/Misc/NEWS.d/next/C API/2017-11-30-18-13-45.bpo-20891.wBnMdF.rst b/Misc/NEWS.d/next/C API/2017-11-30-18-13-45.bpo-20891.wBnMdF.rst new file mode 100644 index 00000000000..e89cf1292af --- /dev/null +++ b/Misc/NEWS.d/next/C API/2017-11-30-18-13-45.bpo-20891.wBnMdF.rst @@ -0,0 +1,3 @@ +Fix PyGILState_Ensure(). When PyGILState_Ensure() is called in a non-Python +thread before PyEval_InitThreads(), only call PyEval_InitThreads() after +calling PyThreadState_New() to fix a crash. diff --git a/Programs/_testembed.c b/Programs/_testembed.c index 39683993ea0..b0f9087115b 100644 --- a/Programs/_testembed.c +++ b/Programs/_testembed.c @@ -1,4 +1,5 @@ #include <Python.h> +#include "pythread.h" #include <stdio.h> /********************************************************* @@ -33,7 +34,7 @@ static void print_subinterp(void) ); } -static void test_repeated_init_and_subinterpreters(void) +static int test_repeated_init_and_subinterpreters(void) { PyThreadState *mainstate, *substate; #ifdef WITH_THREAD @@ -70,6 +71,7 @@ static void test_repeated_init_and_subinterpreters(void) PyEval_RestoreThread(mainstate); Py_Finalize(); } + return 0; } /***************************************************** @@ -103,7 +105,7 @@ static void check_stdio_details(const char *encoding, const char * errors) Py_Finalize(); } -static void test_forced_io_encoding(void) +static int test_forced_io_encoding(void) { /* Check various combinations */ printf("--- Use defaults ---\n"); @@ -122,19 +124,123 @@ static void test_forced_io_encoding(void) printf("Unexpected success calling Py_SetStandardStreamEncoding"); } Py_Finalize(); + return 0; } -/* Different embedding tests */ -int main(int argc, char *argv[]) + +/********************************************************* + * Test parts of the C-API that work before initialization + *********************************************************/ + +static int test_pre_initialization_api(void) { + /* Leading "./" ensures getpath.c can still find the standard library */ + wchar_t *program = Py_DecodeLocale("./spam", NULL); + if (program == NULL) { + fprintf(stderr, "Fatal error: cannot decode program name\n"); + return 1; + } + Py_SetProgramName(program); - /* TODO: Check the argument string to allow for more test cases */ - if (argc > 1) { - /* For now: assume "forced_io_encoding */ - test_forced_io_encoding(); - } else { - /* Run the original embedding test case by default */ - test_repeated_init_and_subinterpreters(); + Py_Initialize(); + Py_Finalize(); + + PyMem_RawFree(program); + return 0; +} + +static void bpo20891_thread(void *lockp) +{ + PyThread_type_lock lock = *((PyThread_type_lock*)lockp); + + PyGILState_STATE state = PyGILState_Ensure(); + if (!PyGILState_Check()) { + fprintf(stderr, "PyGILState_Check failed!"); + abort(); + } + + PyGILState_Release(state); + + PyThread_release_lock(lock); + + PyThread_exit_thread(); +} + +static int test_bpo20891(void) +{ + /* bpo-20891: Calling PyGILState_Ensure in a non-Python thread before + calling PyEval_InitThreads() must not crash. PyGILState_Ensure() must + call PyEval_InitThreads() for us in this case. */ + PyThread_type_lock lock = PyThread_allocate_lock(); + if (!lock) { + fprintf(stderr, "PyThread_allocate_lock failed!"); + return 1; + } + + _testembed_Py_Initialize(); + + long thrd = PyThread_start_new_thread(bpo20891_thread, &lock); + if (thrd == -1) { + fprintf(stderr, "PyThread_start_new_thread failed!"); + return 1; } + PyThread_acquire_lock(lock, WAIT_LOCK); + + Py_BEGIN_ALLOW_THREADS + /* wait until the thread exit */ + PyThread_acquire_lock(lock, WAIT_LOCK); + Py_END_ALLOW_THREADS + + PyThread_free_lock(lock); + return 0; } + + +/* ********************************************************* + * List of test cases and the function that implements it. + * + * Names are compared case-sensitively with the first + * argument. If no match is found, or no first argument was + * provided, the names of all test cases are printed and + * the exit code will be -1. + * + * The int returned from test functions is used as the exit + * code, and test_capi treats all non-zero exit codes as a + * failed test. + *********************************************************/ +struct TestCase +{ + const char *name; + int (*func)(void); +}; + +static struct TestCase TestCases[] = { + { "forced_io_encoding", test_forced_io_encoding }, + { "repeated_init_and_subinterpreters", test_repeated_init_and_subinterpreters }, + { "pre_initialization_api", test_pre_initialization_api }, + { "bpo20891", test_bpo20891 }, + { NULL, NULL } +}; + +int main(int argc, char *argv[]) +{ + if (argc > 1) { + for (struct TestCase *tc = TestCases; tc && tc->name; tc++) { + if (strcmp(argv[1], tc->name) == 0) + return (*tc->func)(); + } + } + + /* No match found, or no test name provided, so display usage */ + printf("Python " PY_VERSION " _testembed executable for embedded interpreter tests\n" + "Normally executed via 'EmbeddingTests' in Lib/test/test_capi.py\n\n" + "Usage: %s TESTNAME\n\nAll available tests:\n", argv[0]); + for (struct TestCase *tc = TestCases; tc && tc->name; tc++) { + printf(" %s\n", tc->name); + } + + /* Non-zero exit code will cause test_capi.py tests to fail. + This is intentional. */ + return -1; +} diff --git a/Python/pystate.c b/Python/pystate.c index 65f9b7ea05d..c0e088055a6 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -865,6 +865,8 @@ PyGILState_Ensure(void) { int current; PyThreadState *tcur; + int need_init_threads = 0; + /* Note that we do not auto-init Python here - apart from potential races with 2 threads auto-initializing, pep-311 spells out other issues. Embedders are expected to have @@ -873,10 +875,7 @@ PyGILState_Ensure(void) assert(autoInterpreterState); /* Py_Initialize() hasn't been called! */ tcur = (PyThreadState *)PyThread_get_key_value(autoTLSkey); if (tcur == NULL) { - /* At startup, Python has no concrete GIL. If PyGILState_Ensure() is - called from a new thread for the first time, we need the create the - GIL. */ - PyEval_InitThreads(); + need_init_threads = 1; /* Create a new thread state for this thread */ tcur = PyThreadState_New(autoInterpreterState); @@ -887,16 +886,28 @@ PyGILState_Ensure(void) tcur->gilstate_counter = 0; current = 0; /* new thread state is never current */ } - else + else { current = PyThreadState_IsCurrent(tcur); - if (current == 0) + } + + if (current == 0) { PyEval_RestoreThread(tcur); + } + /* Update our counter in the thread-state - no need for locks: - tcur will remain valid as we hold the GIL. - the counter is safe as we are the only thread "allowed" to modify this value */ ++tcur->gilstate_counter; + + if (need_init_threads) { + /* At startup, Python has no concrete GIL. If PyGILState_Ensure() is + called from a new thread for the first time, we need the create the + GIL. */ + PyEval_InitThreads(); + } + return current ? PyGILState_LOCKED : PyGILState_UNLOCKED; }

1 0

bpo-20891: Fix PyGILState_Ensure() (#4650) (#4657)
by Victor Stinner 30 Nov '17

30 Nov '17

https://github.com/python/cpython/commit/be6b74c0795b709c7a04e2187a7e32d08f… commit: be6b74c0795b709c7a04e2187a7e32d08f5155f6 branch: 2.7 author: Victor Stinner <victor.stinner(a)gmail.com> committer: GitHub <noreply(a)github.com> date: 2017-11-30T23:35:14+01:00 summary: bpo-20891: Fix PyGILState_Ensure() (#4650) (#4657) When PyGILState_Ensure() is called in a non-Python thread before PyEval_InitThreads(), only call PyEval_InitThreads() after calling PyThreadState_New() to fix a crash. (cherry picked from commit b4d1e1f7c1af6ae33f0e371576c8bcafedb099db) files: A Misc/NEWS.d/next/C API/2017-11-30-18-13-45.bpo-20891.wBnMdF.rst M Python/pystate.c diff --git a/Misc/NEWS.d/next/C API/2017-11-30-18-13-45.bpo-20891.wBnMdF.rst b/Misc/NEWS.d/next/C API/2017-11-30-18-13-45.bpo-20891.wBnMdF.rst new file mode 100644 index 00000000000..e89cf1292af --- /dev/null +++ b/Misc/NEWS.d/next/C API/2017-11-30-18-13-45.bpo-20891.wBnMdF.rst @@ -0,0 +1,3 @@ +Fix PyGILState_Ensure(). When PyGILState_Ensure() is called in a non-Python +thread before PyEval_InitThreads(), only call PyEval_InitThreads() after +calling PyThreadState_New() to fix a crash. diff --git a/Python/pystate.c b/Python/pystate.c index eb992c17801..f33f1820236 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -588,6 +588,8 @@ PyGILState_Ensure(void) { int current; PyThreadState *tcur; + int need_init_threads = 0; + /* Note that we do not auto-init Python here - apart from potential races with 2 threads auto-initializing, pep-311 spells out other issues. Embedders are expected to have @@ -596,6 +598,8 @@ PyGILState_Ensure(void) assert(autoInterpreterState); /* Py_Initialize() hasn't been called! */ tcur = (PyThreadState *)PyThread_get_key_value(autoTLSkey); if (tcur == NULL) { + need_init_threads = 1; + /* Create a new thread state for this thread */ tcur = PyThreadState_New(autoInterpreterState); if (tcur == NULL) @@ -605,16 +609,28 @@ PyGILState_Ensure(void) tcur->gilstate_counter = 0; current = 0; /* new thread state is never current */ } - else + else { current = PyThreadState_IsCurrent(tcur); - if (current == 0) + } + + if (current == 0) { PyEval_RestoreThread(tcur); + } + /* Update our counter in the thread-state - no need for locks: - tcur will remain valid as we hold the GIL. - the counter is safe as we are the only thread "allowed" to modify this value */ ++tcur->gilstate_counter; + + if (need_init_threads) { + /* At startup, Python has no concrete GIL. If PyGILState_Ensure() is + called from a new thread for the first time, we need the create the + GIL. */ + PyEval_InitThreads(); + } + return current ? PyGILState_LOCKED : PyGILState_UNLOCKED; }

1 0

[3.6] make tags: index also Modules/_ctypes/ (#4648) (#4659)
by Victor Stinner 30 Nov '17

30 Nov '17

https://github.com/python/cpython/commit/29cb50ba347d9dc18e0720bef8e9caedd0… commit: 29cb50ba347d9dc18e0720bef8e9caedd012a3cd branch: 3.6 author: Victor Stinner <victor.stinner(a)gmail.com> committer: GitHub <noreply(a)github.com> date: 2017-11-30T23:34:21+01:00 summary: [3.6] make tags: index also Modules/_ctypes/ (#4648) (#4659) * `make tags` fixes (GH-717) * Fix `make tags` warnings `make tags` target tries to find C sources and headers in "Grammar" and "Mac" folders and generates these warnings: ctags: Warning: cannot open source file "Grammar/*.[ch]" : No such file or directory ctags: Warning: cannot open source file "Mac/*.[ch]" : No such file or directory This commit changes $SRCDIRS variable in configure.ac to remote these directories. This variable is used only for tags generation. Also, "configure" was regenerated with `autoreconf`. * Fix `make tags` fail on non-default tag names When ctags overrides default tags filename (e.g. `-f .tags`) `make tags` is failed because it assumes to see default `tags` filename: sort: cannot read: tags: No such file or directory This commit explicitly specifies "tags" filename for tags generation. (cherry picked from commit 8a543c0bc7347d5b333f334d157bf4a7cd33c14a) * make tags: index also Modules/_ctypes/ (#4648) Avoid also "cd $(srcdir)" to not change the current directory. (cherry picked from commit 3be3b97a9709d3cd5303175ddbffa7dcca57ac3e) files: M Makefile.pre.in M configure M configure.ac diff --git a/Makefile.pre.in b/Makefile.pre.in index 2b647a8e588..5e88e3f6aa1 100644 --- a/Makefile.pre.in +++ b/Makefile.pre.in @@ -1599,10 +1599,9 @@ autoconf: # Create a tags file for vi tags:: - cd $(srcdir); \ - ctags -w Include/*.h; \ - for i in $(SRCDIRS); do ctags -w -a $$i/*.[ch]; \ - done; \ + ctags -w $(srcdir)/Include/*.h + for i in $(SRCDIRS); do ctags -f tags -w -a $(srcdir)/$$i/*.[ch]; done + ctags -f tags -w -a $(srcdir)/Modules/_ctypes/*.[ch] LC_ALL=C sort -o tags tags # Create a tags file for GNU Emacs diff --git a/configure b/configure index 24d86f59ebc..ea1baef55d3 100755 --- a/configure +++ b/configure @@ -16553,7 +16553,7 @@ do done -SRCDIRS="Parser Grammar Objects Python Modules Mac Programs" +SRCDIRS="Parser Objects Python Modules Programs" { $as_echo "$as_me:${as_lineno-$LINENO}: checking for build directories" >&5 $as_echo_n "checking for build directories... " >&6; } for dir in $SRCDIRS; do diff --git a/configure.ac b/configure.ac index 9a784e4636f..fc1dba6b029 100644 --- a/configure.ac +++ b/configure.ac @@ -5302,7 +5302,7 @@ do done AC_SUBST(SRCDIRS) -SRCDIRS="Parser Grammar Objects Python Modules Mac Programs" +SRCDIRS="Parser Objects Python Modules Programs" AC_MSG_CHECKING(for build directories) for dir in $SRCDIRS; do if test ! -d $dir; then

1 0

make tags: index also Modules/_ctypes/ (#4648) (#4660)
by Victor Stinner 30 Nov '17

30 Nov '17

https://github.com/python/cpython/commit/7d1cfeef6486af07de832b3eae27e3f635… commit: 7d1cfeef6486af07de832b3eae27e3f63532c270 branch: 2.7 author: Victor Stinner <victor.stinner(a)gmail.com> committer: GitHub <noreply(a)github.com> date: 2017-11-30T23:28:01+01:00 summary: make tags: index also Modules/_ctypes/ (#4648) (#4660) Avoid also "cd $(srcdir)" to not change the current directory. (cherry picked from commit 3be3b97a9709d3cd5303175ddbffa7dcca57ac3e) files: M Makefile.pre.in diff --git a/Makefile.pre.in b/Makefile.pre.in index 19e8f55b2ab..9297e7fc89c 100644 --- a/Makefile.pre.in +++ b/Makefile.pre.in @@ -1378,10 +1378,9 @@ autoconf: # Create a tags file for vi tags:: - cd $(srcdir); \ - ctags -w Include/*.h; \ - for i in $(SRCDIRS); do ctags -f tags -w -a $$i/*.[ch]; \ - done; \ + ctags -w $(srcdir)/Include/*.h + for i in $(SRCDIRS); do ctags -f tags -w -a $(srcdir)/$$i/*.[ch]; done + ctags -f tags -w -a $(srcdir)/Modules/_ctypes/*.[ch] LC_ALL=C sort -o tags tags # Create a tags file for GNU Emacs

1 0

bpo-32072: Fix issues with binary plists. (GH-4455) (#4654)
by Serhiy Storchaka 30 Nov '17

30 Nov '17

https://github.com/python/cpython/commit/8cd31082fba88cf0064590fd3d55b6c1c9… commit: 8cd31082fba88cf0064590fd3d55b6c1c964f11c branch: 3.6 author: Miss Islington (bot) <31488909+miss-islington(a)users.noreply.github.com> committer: Serhiy Storchaka <storchaka(a)gmail.com> date: 2017-12-01T00:15:30+02:00 summary: bpo-32072: Fix issues with binary plists. (GH-4455) (#4654) * Fixed saving bytearrays. * Identical objects will be saved only once. * Equal references will be load as identical objects. * Added support for saving and loading recursive data structures. (cherry picked from commit a897aeeef647259a938a36cb5eb6680c86021c6a) files: A Misc/NEWS.d/next/Library/2017-11-18-21-13-52.bpo-32072.nwDV8L.rst M Lib/plistlib.py M Lib/test/test_plistlib.py diff --git a/Lib/plistlib.py b/Lib/plistlib.py index 9cfc7949681..a9186437255 100644 --- a/Lib/plistlib.py +++ b/Lib/plistlib.py @@ -590,6 +590,8 @@ def __init__(self, message="Invalid file"): _BINARY_FORMAT = {1: 'B', 2: 'H', 4: 'L', 8: 'Q'} +_undefined = object() + class _BinaryPlistParser: """ Read or write a binary plist file, following the description of the binary @@ -620,7 +622,8 @@ def parse(self, fp): ) = struct.unpack('>6xBBQQQ', trailer) self._fp.seek(offset_table_offset) self._object_offsets = self._read_ints(num_objects, offset_size) - return self._read_object(self._object_offsets[top_object]) + self._objects = [_undefined] * num_objects + return self._read_object(top_object) except (OSError, IndexError, struct.error, OverflowError, UnicodeDecodeError): @@ -649,62 +652,68 @@ def _read_ints(self, n, size): def _read_refs(self, n): return self._read_ints(n, self._ref_size) - def _read_object(self, offset): + def _read_object(self, ref): """ - read the object at offset. + read the object by reference. May recursively read sub-objects (content of an array/dict/set) """ + result = self._objects[ref] + if result is not _undefined: + return result + + offset = self._object_offsets[ref] self._fp.seek(offset) token = self._fp.read(1)[0] tokenH, tokenL = token & 0xF0, token & 0x0F if token == 0x00: - return None + result = None elif token == 0x08: - return False + result = False elif token == 0x09: - return True + result = True # The referenced source code also mentions URL (0x0c, 0x0d) and # UUID (0x0e), but neither can be generated using the Cocoa libraries. elif token == 0x0f: - return b'' + result = b'' elif tokenH == 0x10: # int - return int.from_bytes(self._fp.read(1 << tokenL), - 'big', signed=tokenL >= 3) + result = int.from_bytes(self._fp.read(1 << tokenL), + 'big', signed=tokenL >= 3) elif token == 0x22: # real - return struct.unpack('>f', self._fp.read(4))[0] + result = struct.unpack('>f', self._fp.read(4))[0] elif token == 0x23: # real - return struct.unpack('>d', self._fp.read(8))[0] + result = struct.unpack('>d', self._fp.read(8))[0] elif token == 0x33: # date f = struct.unpack('>d', self._fp.read(8))[0] # timestamp 0 of binary plists corresponds to 1/1/2001 # (year of Mac OS X 10.0), instead of 1/1/1970. - return datetime.datetime(2001, 1, 1) + datetime.timedelta(seconds=f) + result = (datetime.datetime(2001, 1, 1) + + datetime.timedelta(seconds=f)) elif tokenH == 0x40: # data s = self._get_size(tokenL) if self._use_builtin_types: - return self._fp.read(s) + result = self._fp.read(s) else: - return Data(self._fp.read(s)) + result = Data(self._fp.read(s)) elif tokenH == 0x50: # ascii string s = self._get_size(tokenL) result = self._fp.read(s).decode('ascii') - return result + result = result elif tokenH == 0x60: # unicode string s = self._get_size(tokenL) - return self._fp.read(s * 2).decode('utf-16be') + result = self._fp.read(s * 2).decode('utf-16be') # tokenH == 0x80 is documented as 'UID' and appears to be used for # keyed-archiving, not in plists. @@ -712,8 +721,9 @@ def _read_object(self, offset): elif tokenH == 0xA0: # array s = self._get_size(tokenL) obj_refs = self._read_refs(s) - return [self._read_object(self._object_offsets[x]) - for x in obj_refs] + result = [] + self._objects[ref] = result + result.extend(self._read_object(x) for x in obj_refs) # tokenH == 0xB0 is documented as 'ordset', but is not actually # implemented in the Apple reference code. @@ -726,12 +736,15 @@ def _read_object(self, offset): key_refs = self._read_refs(s) obj_refs = self._read_refs(s) result = self._dict_type() + self._objects[ref] = result for k, o in zip(key_refs, obj_refs): - result[self._read_object(self._object_offsets[k]) - ] = self._read_object(self._object_offsets[o]) - return result + result[self._read_object(k)] = self._read_object(o) - raise InvalidFileException() + else: + raise InvalidFileException() + + self._objects[ref] = result + return result def _count_to_size(count): if count < 1 << 8: @@ -746,6 +759,8 @@ def _count_to_size(count): else: return 8 +_scalars = (str, int, float, datetime.datetime, bytes) + class _BinaryPlistWriter (object): def __init__(self, fp, sort_keys, skipkeys): self._fp = fp @@ -801,8 +816,7 @@ def _flatten(self, value): # First check if the object is in the object table, not used for # containers to ensure that two subcontainers with the same contents # will be serialized as distinct values. - if isinstance(value, ( - str, int, float, datetime.datetime, bytes, bytearray)): + if isinstance(value, _scalars): if (type(value), value) in self._objtable: return @@ -810,15 +824,17 @@ def _flatten(self, value): if (type(value.data), value.data) in self._objtable: return + elif id(value) in self._objidtable: + return + # Add to objectreference map refnum = len(self._objlist) self._objlist.append(value) - try: - if isinstance(value, Data): - self._objtable[(type(value.data), value.data)] = refnum - else: - self._objtable[(type(value), value)] = refnum - except TypeError: + if isinstance(value, _scalars): + self._objtable[(type(value), value)] = refnum + elif isinstance(value, Data): + self._objtable[(type(value.data), value.data)] = refnum + else: self._objidtable[id(value)] = refnum # And finally recurse into containers @@ -845,12 +861,11 @@ def _flatten(self, value): self._flatten(o) def _getrefnum(self, value): - try: - if isinstance(value, Data): - return self._objtable[(type(value.data), value.data)] - else: - return self._objtable[(type(value), value)] - except TypeError: + if isinstance(value, _scalars): + return self._objtable[(type(value), value)] + elif isinstance(value, Data): + return self._objtable[(type(value.data), value.data)] + else: return self._objidtable[id(value)] def _write_size(self, token, size): diff --git a/Lib/test/test_plistlib.py b/Lib/test/test_plistlib.py index 4c9ba4c3eaa..90641a7635c 100644 --- a/Lib/test/test_plistlib.py +++ b/Lib/test/test_plistlib.py @@ -169,6 +169,17 @@ def test_int(self): self.assertRaises(OverflowError, plistlib.dumps, pl, fmt=fmt) + def test_bytearray(self): + for pl in (b'<binary gunk>', b"<lots of binary gunk>\0\1\2\3" * 10): + for fmt in ALL_FORMATS: + with self.subTest(pl=pl, fmt=fmt): + data = plistlib.dumps(bytearray(pl), fmt=fmt) + pl2 = plistlib.loads(data) + self.assertIsInstance(pl2, bytes) + self.assertEqual(pl2, pl) + data2 = plistlib.dumps(pl2, fmt=fmt) + self.assertEqual(data, data2) + def test_bytes(self): pl = self._create() data = plistlib.dumps(pl) @@ -432,6 +443,9 @@ def test_xml_encodings(self): pl2 = plistlib.loads(data) self.assertEqual(dict(pl), dict(pl2)) + +class TestBinaryPlistlib(unittest.TestCase): + def test_nonstandard_refs_size(self): # Issue #21538: Refs and offsets are 24-bit integers data = (b'bplist00' @@ -444,6 +458,47 @@ def test_nonstandard_refs_size(self): b'\x00\x00\x00\x00\x00\x00\x00\x13') self.assertEqual(plistlib.loads(data), {'a': 'b'}) + def test_dump_duplicates(self): + # Test effectiveness of saving duplicated objects + for x in (None, False, True, 12345, 123.45, 'abcde', b'abcde', + datetime.datetime(2004, 10, 26, 10, 33, 33), + plistlib.Data(b'abcde'), bytearray(b'abcde'), + [12, 345], (12, 345), {'12': 345}): + with self.subTest(x=x): + data = plistlib.dumps([x]*1000, fmt=plistlib.FMT_BINARY) + self.assertLess(len(data), 1100, repr(data)) + + def test_identity(self): + for x in (None, False, True, 12345, 123.45, 'abcde', b'abcde', + datetime.datetime(2004, 10, 26, 10, 33, 33), + plistlib.Data(b'abcde'), bytearray(b'abcde'), + [12, 345], (12, 345), {'12': 345}): + with self.subTest(x=x): + data = plistlib.dumps([x]*2, fmt=plistlib.FMT_BINARY) + a, b = plistlib.loads(data) + if isinstance(x, tuple): + x = list(x) + self.assertEqual(a, x) + self.assertEqual(b, x) + self.assertIs(a, b) + + def test_cycles(self): + # recursive list + a = [] + a.append(a) + b = plistlib.loads(plistlib.dumps(a, fmt=plistlib.FMT_BINARY)) + self.assertIs(b[0], b) + # recursive tuple + a = ([],) + a[0].append(a) + b = plistlib.loads(plistlib.dumps(a, fmt=plistlib.FMT_BINARY)) + self.assertIs(b[0][0], b) + # recursive dict + a = {} + a['x'] = a + b = plistlib.loads(plistlib.dumps(a, fmt=plistlib.FMT_BINARY)) + self.assertIs(b['x'], b) + def test_large_timestamp(self): # Issue #26709: 32-bit timestamp out of range for ts in -2**31-1, 2**31: diff --git a/Misc/NEWS.d/next/Library/2017-11-18-21-13-52.bpo-32072.nwDV8L.rst b/Misc/NEWS.d/next/Library/2017-11-18-21-13-52.bpo-32072.nwDV8L.rst new file mode 100644 index 00000000000..6da5bb427d4 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2017-11-18-21-13-52.bpo-32072.nwDV8L.rst @@ -0,0 +1,6 @@ +Fixed issues with binary plists: + +* Fixed saving bytearrays. +* Identical objects will be saved only once. +* Equal references will be load as identical objects. +* Added support for saving and loading recursive data structures.

1 0

Fix CID-1420310: cast PY_TIMEOUT_MAX to _Py_time_t (#4646)
by Victor Stinner 30 Nov '17

30 Nov '17

https://github.com/python/cpython/commit/c319eeeb45043ee45384b8064c53ddbfde… commit: c319eeeb45043ee45384b8064c53ddbfde1673cd branch: master author: Victor Stinner <victor.stinner(a)gmail.com> committer: GitHub <noreply(a)github.com> date: 2017-11-30T23:03:47+01:00 summary: Fix CID-1420310: cast PY_TIMEOUT_MAX to _Py_time_t (#4646) Fix the following false-alarm Coverity warning: Result is not floating-point (UNINTENDED_INTEGER_DIVISION)integer_division: Dividing integer expressions 9223372036854775807LL and 1000LL, and then converting the integer quotient to type double. Any remainder, or fractional part of the quotient, is ignored. To compute and use a non-integer quotient, change or cast either operand to type double. If integer division is intended, consider indicating that by casting the result to type long long . files: M Modules/_threadmodule.c diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index aaa13da890b..c9171f52e38 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1363,7 +1363,7 @@ PyInit__thread(void) if (m == NULL) return NULL; - timeout_max = (double)PY_TIMEOUT_MAX * 1e-6; + timeout_max = (_PyTime_t)PY_TIMEOUT_MAX * 1e-6; time_max = _PyTime_AsSecondsDouble(_PyTime_MAX); timeout_max = Py_MIN(timeout_max, time_max); /* Round towards minus infinity */

1 0

make tags: index also Modules/_ctypes/ (#4648)
by Victor Stinner 30 Nov '17

30 Nov '17

https://github.com/python/cpython/commit/3be3b97a9709d3cd5303175ddbffa7dcca… commit: 3be3b97a9709d3cd5303175ddbffa7dcca57ac3e branch: master author: Victor Stinner <victor.stinner(a)gmail.com> committer: GitHub <noreply(a)github.com> date: 2017-11-30T22:49:10+01:00 summary: make tags: index also Modules/_ctypes/ (#4648) Avoid also "cd $(srcdir)" to not change the current directory. files: M Makefile.pre.in diff --git a/Makefile.pre.in b/Makefile.pre.in index d196d5f838e..f425a89173a 100644 --- a/Makefile.pre.in +++ b/Makefile.pre.in @@ -1605,10 +1605,9 @@ autoconf: # Create a tags file for vi tags:: - cd $(srcdir); \ - ctags -w Include/*.h Include/internal/*.h; \ - for i in $(SRCDIRS); do ctags -f tags -w -a $$i/*.[ch]; \ - done; \ + ctags -w $(srcdir)/Include/*.h $(srcdir)/Include/internal/*.h + for i in $(SRCDIRS); do ctags -f tags -w -a $(srcdir)/$$i/*.[ch]; done + ctags -f tags -w -a $(srcdir)/Modules/_ctypes/*.[ch] LC_ALL=C sort -o tags tags # Create a tags file for GNU Emacs

1 0

bpo-28416: Break reference cycles in Pickler and Unpickler subclasses (GH-4080) (#4653)
by Serhiy Storchaka 30 Nov '17

30 Nov '17

https://github.com/python/cpython/commit/c91bf742e542dceaf71042a44b5a04fb08… commit: c91bf742e542dceaf71042a44b5a04fb08bdda70 branch: 3.6 author: Miss Islington (bot) <31488909+miss-islington(a)users.noreply.github.com> committer: Serhiy Storchaka <storchaka(a)gmail.com> date: 2017-11-30T23:30:39+02:00 summary: bpo-28416: Break reference cycles in Pickler and Unpickler subclasses (GH-4080) (#4653) with the persistent_id() and persistent_load() methods. (cherry picked from commit 986375ebde0dd5ff2b7349e445a06bd28a3a8ee2) files: A Misc/NEWS.d/next/Library/2017-10-23-12-05-33.bpo-28416.Ldnw8X.rst M Lib/test/test_pickle.py M Modules/_pickle.c diff --git a/Lib/test/test_pickle.py b/Lib/test/test_pickle.py index ee71c632733..895ed48df1d 100644 --- a/Lib/test/test_pickle.py +++ b/Lib/test/test_pickle.py @@ -6,6 +6,7 @@ import collections import struct import sys +import weakref import unittest from test import support @@ -117,6 +118,66 @@ class PyIdPersPicklerTests(AbstractIdentityPersistentPicklerTests, pickler = pickle._Pickler unpickler = pickle._Unpickler + @support.cpython_only + def test_pickler_reference_cycle(self): + def check(Pickler): + for proto in range(pickle.HIGHEST_PROTOCOL + 1): + f = io.BytesIO() + pickler = Pickler(f, proto) + pickler.dump('abc') + self.assertEqual(self.loads(f.getvalue()), 'abc') + pickler = Pickler(io.BytesIO()) + self.assertEqual(pickler.persistent_id('def'), 'def') + r = weakref.ref(pickler) + del pickler + self.assertIsNone(r()) + + class PersPickler(self.pickler): + def persistent_id(subself, obj): + return obj + check(PersPickler) + + class PersPickler(self.pickler): + @classmethod + def persistent_id(cls, obj): + return obj + check(PersPickler) + + class PersPickler(self.pickler): + @staticmethod + def persistent_id(obj): + return obj + check(PersPickler) + + @support.cpython_only + def test_unpickler_reference_cycle(self): + def check(Unpickler): + for proto in range(pickle.HIGHEST_PROTOCOL + 1): + unpickler = Unpickler(io.BytesIO(self.dumps('abc', proto))) + self.assertEqual(unpickler.load(), 'abc') + unpickler = Unpickler(io.BytesIO()) + self.assertEqual(unpickler.persistent_load('def'), 'def') + r = weakref.ref(unpickler) + del unpickler + self.assertIsNone(r()) + + class PersUnpickler(self.unpickler): + def persistent_load(subself, pid): + return pid + check(PersUnpickler) + + class PersUnpickler(self.unpickler): + @classmethod + def persistent_load(cls, pid): + return pid + check(PersUnpickler) + + class PersUnpickler(self.unpickler): + @staticmethod + def persistent_load(pid): + return pid + check(PersUnpickler) + class PyPicklerUnpicklerObjectTests(AbstractPicklerUnpicklerObjectTests): @@ -197,7 +258,7 @@ class SizeofTests(unittest.TestCase): check_sizeof = support.check_sizeof def test_pickler(self): - basesize = support.calcobjsize('5P2n3i2n3iP') + basesize = support.calcobjsize('6P2n3i2n3iP') p = _pickle.Pickler(io.BytesIO()) self.assertEqual(object.__sizeof__(p), basesize) MT_size = struct.calcsize('3nP0n') @@ -214,7 +275,7 @@ def test_pickler(self): 0) # Write buffer is cleared after every dump(). def test_unpickler(self): - basesize = support.calcobjsize('2Pn2P 2P2n2i5P 2P3n6P2n2i') + basesize = support.calcobjsize('2P2n2P 2P2n2i5P 2P3n6P2n2i') unpickler = _pickle.Unpickler P = struct.calcsize('P') # Size of memo table entry. n = struct.calcsize('n') # Size of mark table entry. diff --git a/Misc/NEWS.d/next/Library/2017-10-23-12-05-33.bpo-28416.Ldnw8X.rst b/Misc/NEWS.d/next/Library/2017-10-23-12-05-33.bpo-28416.Ldnw8X.rst new file mode 100644 index 00000000000..b1014827af6 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2017-10-23-12-05-33.bpo-28416.Ldnw8X.rst @@ -0,0 +1,3 @@ +Instances of pickle.Pickler subclass with the persistent_id() method and +pickle.Unpickler subclass with the persistent_load() method no longer create +reference cycles. diff --git a/Modules/_pickle.c b/Modules/_pickle.c index 198474d88bf..7e9bb9895ca 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -353,6 +353,69 @@ _Pickle_FastCall(PyObject *func, PyObject *obj) /*************************************************************************/ +/* Retrieve and deconstruct a method for avoiding a reference cycle + (pickler -> bound method of pickler -> pickler) */ +static int +init_method_ref(PyObject *self, _Py_Identifier *name, + PyObject **method_func, PyObject **method_self) +{ + PyObject *func, *func2; + + /* *method_func and *method_self should be consistent. All refcount decrements + should be occurred after setting *method_self and *method_func. */ + func = _PyObject_GetAttrId(self, name); + if (func == NULL) { + *method_self = NULL; + Py_CLEAR(*method_func); + if (!PyErr_ExceptionMatches(PyExc_AttributeError)) { + return -1; + } + PyErr_Clear(); + return 0; + } + + if (PyMethod_Check(func) && PyMethod_GET_SELF(func) == self) { + /* Deconstruct a bound Python method */ + func2 = PyMethod_GET_FUNCTION(func); + Py_INCREF(func2); + *method_self = self; /* borrowed */ + Py_XSETREF(*method_func, func2); + Py_DECREF(func); + return 0; + } + else { + *method_self = NULL; + Py_XSETREF(*method_func, func); + return 0; + } +} + +/* Bind a method if it was deconstructed */ +static PyObject * +reconstruct_method(PyObject *func, PyObject *self) +{ + if (self) { + return PyMethod_New(func, self); + } + else { + Py_INCREF(func); + return func; + } +} + +static PyObject * +call_method(PyObject *func, PyObject *self, PyObject *obj) +{ + if (self) { + return PyObject_CallFunctionObjArgs(func, self, obj, NULL); + } + else { + return PyObject_CallFunctionObjArgs(func, obj, NULL); + } +} + +/*************************************************************************/ + /* Internal data type used as the unpickling stack. */ typedef struct { PyObject_VAR_HEAD @@ -545,6 +608,8 @@ typedef struct PicklerObject { objects to support self-referential objects pickling. */ PyObject *pers_func; /* persistent_id() method, can be NULL */ + PyObject *pers_func_self; /* borrowed reference to self if pers_func + is an unbound method, NULL otherwise */ PyObject *dispatch_table; /* private dispatch_table, can be NULL */ PyObject *write; /* write() method of the output stream. */ @@ -583,6 +648,8 @@ typedef struct UnpicklerObject { Py_ssize_t memo_len; /* Number of objects in the memo */ PyObject *pers_func; /* persistent_load() method, can be NULL. */ + PyObject *pers_func_self; /* borrowed reference to self if pers_func + is an unbound method, NULL otherwise */ Py_buffer buffer; char *input_buffer; @@ -3401,7 +3468,7 @@ save_type(PicklerObject *self, PyObject *obj) } static int -save_pers(PicklerObject *self, PyObject *obj, PyObject *func) +save_pers(PicklerObject *self, PyObject *obj) { PyObject *pid = NULL; int status = 0; @@ -3409,8 +3476,7 @@ save_pers(PicklerObject *self, PyObject *obj, PyObject *func) const char persid_op = PERSID; const char binpersid_op = BINPERSID; - Py_INCREF(obj); - pid = _Pickle_FastCall(func, obj); + pid = call_method(self->pers_func, self->pers_func_self, obj); if (pid == NULL) return -1; @@ -3788,7 +3854,7 @@ save(PicklerObject *self, PyObject *obj, int pers_save) 0 if it did nothing successfully; 1 if a persistent id was saved. */ - if ((status = save_pers(self, obj, self->pers_func)) != 0) + if ((status = save_pers(self, obj)) != 0) goto done; } @@ -4203,13 +4269,10 @@ _pickle_Pickler___init___impl(PicklerObject *self, PyObject *file, self->fast_nesting = 0; self->fast_memo = NULL; - self->pers_func = _PyObject_GetAttrId((PyObject *)self, - &PyId_persistent_id); - if (self->pers_func == NULL) { - if (!PyErr_ExceptionMatches(PyExc_AttributeError)) { - return -1; - } - PyErr_Clear(); + if (init_method_ref((PyObject *)self, &PyId_persistent_id, + &self->pers_func, &self->pers_func_self) < 0) + { + return -1; } self->dispatch_table = _PyObject_GetAttrId((PyObject *)self, @@ -4476,11 +4539,11 @@ Pickler_set_memo(PicklerObject *self, PyObject *obj) static PyObject * Pickler_get_persid(PicklerObject *self) { - if (self->pers_func == NULL) + if (self->pers_func == NULL) { PyErr_SetString(PyExc_AttributeError, "persistent_id"); - else - Py_INCREF(self->pers_func); - return self->pers_func; + return NULL; + } + return reconstruct_method(self->pers_func, self->pers_func_self); } static int @@ -4497,6 +4560,7 @@ Pickler_set_persid(PicklerObject *self, PyObject *value) return -1; } + self->pers_func_self = NULL; Py_INCREF(value); Py_XSETREF(self->pers_func, value); @@ -5446,7 +5510,7 @@ load_stack_global(UnpicklerObject *self) static int load_persid(UnpicklerObject *self) { - PyObject *pid; + PyObject *pid, *obj; Py_ssize_t len; char *s; @@ -5466,13 +5530,12 @@ load_persid(UnpicklerObject *self) return -1; } - /* This does not leak since _Pickle_FastCall() steals the reference - to pid first. */ - pid = _Pickle_FastCall(self->pers_func, pid); - if (pid == NULL) + obj = call_method(self->pers_func, self->pers_func_self, pid); + Py_DECREF(pid); + if (obj == NULL) return -1; - PDATA_PUSH(self->stack, pid, -1); + PDATA_PUSH(self->stack, obj, -1); return 0; } else { @@ -5487,20 +5550,19 @@ load_persid(UnpicklerObject *self) static int load_binpersid(UnpicklerObject *self) { - PyObject *pid; + PyObject *pid, *obj; if (self->pers_func) { PDATA_POP(self->stack, pid); if (pid == NULL) return -1; - /* This does not leak since _Pickle_FastCall() steals the - reference to pid first. */ - pid = _Pickle_FastCall(self->pers_func, pid); - if (pid == NULL) + obj = call_method(self->pers_func, self->pers_func_self, pid); + Py_DECREF(pid); + if (obj == NULL) return -1; - PDATA_PUSH(self->stack, pid, -1); + PDATA_PUSH(self->stack, obj, -1); return 0; } else { @@ -6637,13 +6699,10 @@ _pickle_Unpickler___init___impl(UnpicklerObject *self, PyObject *file, self->fix_imports = fix_imports; - self->pers_func = _PyObject_GetAttrId((PyObject *)self, - &PyId_persistent_load); - if (self->pers_func == NULL) { - if (!PyErr_ExceptionMatches(PyExc_AttributeError)) { - return -1; - } - PyErr_Clear(); + if (init_method_ref((PyObject *)self, &PyId_persistent_load, + &self->pers_func, &self->pers_func_self) < 0) + { + return -1; } self->stack = (Pdata *)Pdata_New(); @@ -6930,11 +6989,11 @@ Unpickler_set_memo(UnpicklerObject *self, PyObject *obj) static PyObject * Unpickler_get_persload(UnpicklerObject *self) { - if (self->pers_func == NULL) + if (self->pers_func == NULL) { PyErr_SetString(PyExc_AttributeError, "persistent_load"); - else - Py_INCREF(self->pers_func); - return self->pers_func; + return NULL; + } + return reconstruct_method(self->pers_func, self->pers_func_self); } static int @@ -6952,6 +7011,7 @@ Unpickler_set_persload(UnpicklerObject *self, PyObject *value) return -1; } + self->pers_func_self = NULL; Py_INCREF(value); Py_XSETREF(self->pers_func, value);

1 0

bpo-32072: Fix issues with binary plists. (#4455)
by Serhiy Storchaka 30 Nov '17

30 Nov '17

https://github.com/python/cpython/commit/a897aeeef647259a938a36cb5eb6680c86… commit: a897aeeef647259a938a36cb5eb6680c86021c6a branch: master author: Serhiy Storchaka <storchaka(a)gmail.com> committer: GitHub <noreply(a)github.com> date: 2017-11-30T23:26:11+02:00 summary: bpo-32072: Fix issues with binary plists. (#4455) * Fixed saving bytearrays. * Identical objects will be saved only once. * Equal references will be load as identical objects. * Added support for saving and loading recursive data structures. files: A Misc/NEWS.d/next/Library/2017-11-18-21-13-52.bpo-32072.nwDV8L.rst M Lib/plistlib.py M Lib/test/test_plistlib.py diff --git a/Lib/plistlib.py b/Lib/plistlib.py index 2113a2dc57b..21ebec3f004 100644 --- a/Lib/plistlib.py +++ b/Lib/plistlib.py @@ -525,6 +525,8 @@ def __init__(self, message="Invalid file"): _BINARY_FORMAT = {1: 'B', 2: 'H', 4: 'L', 8: 'Q'} +_undefined = object() + class _BinaryPlistParser: """ Read or write a binary plist file, following the description of the binary @@ -555,7 +557,8 @@ def parse(self, fp): ) = struct.unpack('>6xBBQQQ', trailer) self._fp.seek(offset_table_offset) self._object_offsets = self._read_ints(num_objects, offset_size) - return self._read_object(self._object_offsets[top_object]) + self._objects = [_undefined] * num_objects + return self._read_object(top_object) except (OSError, IndexError, struct.error, OverflowError, UnicodeDecodeError): @@ -584,62 +587,68 @@ def _read_ints(self, n, size): def _read_refs(self, n): return self._read_ints(n, self._ref_size) - def _read_object(self, offset): + def _read_object(self, ref): """ - read the object at offset. + read the object by reference. May recursively read sub-objects (content of an array/dict/set) """ + result = self._objects[ref] + if result is not _undefined: + return result + + offset = self._object_offsets[ref] self._fp.seek(offset) token = self._fp.read(1)[0] tokenH, tokenL = token & 0xF0, token & 0x0F if token == 0x00: - return None + result = None elif token == 0x08: - return False + result = False elif token == 0x09: - return True + result = True # The referenced source code also mentions URL (0x0c, 0x0d) and # UUID (0x0e), but neither can be generated using the Cocoa libraries. elif token == 0x0f: - return b'' + result = b'' elif tokenH == 0x10: # int - return int.from_bytes(self._fp.read(1 << tokenL), - 'big', signed=tokenL >= 3) + result = int.from_bytes(self._fp.read(1 << tokenL), + 'big', signed=tokenL >= 3) elif token == 0x22: # real - return struct.unpack('>f', self._fp.read(4))[0] + result = struct.unpack('>f', self._fp.read(4))[0] elif token == 0x23: # real - return struct.unpack('>d', self._fp.read(8))[0] + result = struct.unpack('>d', self._fp.read(8))[0] elif token == 0x33: # date f = struct.unpack('>d', self._fp.read(8))[0] # timestamp 0 of binary plists corresponds to 1/1/2001 # (year of Mac OS X 10.0), instead of 1/1/1970. - return datetime.datetime(2001, 1, 1) + datetime.timedelta(seconds=f) + result = (datetime.datetime(2001, 1, 1) + + datetime.timedelta(seconds=f)) elif tokenH == 0x40: # data s = self._get_size(tokenL) if self._use_builtin_types: - return self._fp.read(s) + result = self._fp.read(s) else: - return Data(self._fp.read(s)) + result = Data(self._fp.read(s)) elif tokenH == 0x50: # ascii string s = self._get_size(tokenL) result = self._fp.read(s).decode('ascii') - return result + result = result elif tokenH == 0x60: # unicode string s = self._get_size(tokenL) - return self._fp.read(s * 2).decode('utf-16be') + result = self._fp.read(s * 2).decode('utf-16be') # tokenH == 0x80 is documented as 'UID' and appears to be used for # keyed-archiving, not in plists. @@ -647,8 +656,9 @@ def _read_object(self, offset): elif tokenH == 0xA0: # array s = self._get_size(tokenL) obj_refs = self._read_refs(s) - return [self._read_object(self._object_offsets[x]) - for x in obj_refs] + result = [] + self._objects[ref] = result + result.extend(self._read_object(x) for x in obj_refs) # tokenH == 0xB0 is documented as 'ordset', but is not actually # implemented in the Apple reference code. @@ -661,12 +671,15 @@ def _read_object(self, offset): key_refs = self._read_refs(s) obj_refs = self._read_refs(s) result = self._dict_type() + self._objects[ref] = result for k, o in zip(key_refs, obj_refs): - result[self._read_object(self._object_offsets[k]) - ] = self._read_object(self._object_offsets[o]) - return result + result[self._read_object(k)] = self._read_object(o) - raise InvalidFileException() + else: + raise InvalidFileException() + + self._objects[ref] = result + return result def _count_to_size(count): if count < 1 << 8: @@ -681,6 +694,8 @@ def _count_to_size(count): else: return 8 +_scalars = (str, int, float, datetime.datetime, bytes) + class _BinaryPlistWriter (object): def __init__(self, fp, sort_keys, skipkeys): self._fp = fp @@ -736,8 +751,7 @@ def _flatten(self, value): # First check if the object is in the object table, not used for # containers to ensure that two subcontainers with the same contents # will be serialized as distinct values. - if isinstance(value, ( - str, int, float, datetime.datetime, bytes, bytearray)): + if isinstance(value, _scalars): if (type(value), value) in self._objtable: return @@ -745,15 +759,17 @@ def _flatten(self, value): if (type(value.data), value.data) in self._objtable: return + elif id(value) in self._objidtable: + return + # Add to objectreference map refnum = len(self._objlist) self._objlist.append(value) - try: - if isinstance(value, Data): - self._objtable[(type(value.data), value.data)] = refnum - else: - self._objtable[(type(value), value)] = refnum - except TypeError: + if isinstance(value, _scalars): + self._objtable[(type(value), value)] = refnum + elif isinstance(value, Data): + self._objtable[(type(value.data), value.data)] = refnum + else: self._objidtable[id(value)] = refnum # And finally recurse into containers @@ -780,12 +796,11 @@ def _flatten(self, value): self._flatten(o) def _getrefnum(self, value): - try: - if isinstance(value, Data): - return self._objtable[(type(value.data), value.data)] - else: - return self._objtable[(type(value), value)] - except TypeError: + if isinstance(value, _scalars): + return self._objtable[(type(value), value)] + elif isinstance(value, Data): + return self._objtable[(type(value.data), value.data)] + else: return self._objidtable[id(value)] def _write_size(self, token, size): diff --git a/Lib/test/test_plistlib.py b/Lib/test/test_plistlib.py index 7dd179a2c71..ae8b6239690 100644 --- a/Lib/test/test_plistlib.py +++ b/Lib/test/test_plistlib.py @@ -169,6 +169,17 @@ def test_int(self): self.assertRaises(OverflowError, plistlib.dumps, pl, fmt=fmt) + def test_bytearray(self): + for pl in (b'<binary gunk>', b"<lots of binary gunk>\0\1\2\3" * 10): + for fmt in ALL_FORMATS: + with self.subTest(pl=pl, fmt=fmt): + data = plistlib.dumps(bytearray(pl), fmt=fmt) + pl2 = plistlib.loads(data) + self.assertIsInstance(pl2, bytes) + self.assertEqual(pl2, pl) + data2 = plistlib.dumps(pl2, fmt=fmt) + self.assertEqual(data, data2) + def test_bytes(self): pl = self._create() data = plistlib.dumps(pl) @@ -431,6 +442,9 @@ def test_xml_encodings(self): pl2 = plistlib.loads(data) self.assertEqual(dict(pl), dict(pl2)) + +class TestBinaryPlistlib(unittest.TestCase): + def test_nonstandard_refs_size(self): # Issue #21538: Refs and offsets are 24-bit integers data = (b'bplist00' @@ -443,6 +457,47 @@ def test_nonstandard_refs_size(self): b'\x00\x00\x00\x00\x00\x00\x00\x13') self.assertEqual(plistlib.loads(data), {'a': 'b'}) + def test_dump_duplicates(self): + # Test effectiveness of saving duplicated objects + for x in (None, False, True, 12345, 123.45, 'abcde', b'abcde', + datetime.datetime(2004, 10, 26, 10, 33, 33), + plistlib.Data(b'abcde'), bytearray(b'abcde'), + [12, 345], (12, 345), {'12': 345}): + with self.subTest(x=x): + data = plistlib.dumps([x]*1000, fmt=plistlib.FMT_BINARY) + self.assertLess(len(data), 1100, repr(data)) + + def test_identity(self): + for x in (None, False, True, 12345, 123.45, 'abcde', b'abcde', + datetime.datetime(2004, 10, 26, 10, 33, 33), + plistlib.Data(b'abcde'), bytearray(b'abcde'), + [12, 345], (12, 345), {'12': 345}): + with self.subTest(x=x): + data = plistlib.dumps([x]*2, fmt=plistlib.FMT_BINARY) + a, b = plistlib.loads(data) + if isinstance(x, tuple): + x = list(x) + self.assertEqual(a, x) + self.assertEqual(b, x) + self.assertIs(a, b) + + def test_cycles(self): + # recursive list + a = [] + a.append(a) + b = plistlib.loads(plistlib.dumps(a, fmt=plistlib.FMT_BINARY)) + self.assertIs(b[0], b) + # recursive tuple + a = ([],) + a[0].append(a) + b = plistlib.loads(plistlib.dumps(a, fmt=plistlib.FMT_BINARY)) + self.assertIs(b[0][0], b) + # recursive dict + a = {} + a['x'] = a + b = plistlib.loads(plistlib.dumps(a, fmt=plistlib.FMT_BINARY)) + self.assertIs(b['x'], b) + def test_large_timestamp(self): # Issue #26709: 32-bit timestamp out of range for ts in -2**31-1, 2**31: diff --git a/Misc/NEWS.d/next/Library/2017-11-18-21-13-52.bpo-32072.nwDV8L.rst b/Misc/NEWS.d/next/Library/2017-11-18-21-13-52.bpo-32072.nwDV8L.rst new file mode 100644 index 00000000000..6da5bb427d4 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2017-11-18-21-13-52.bpo-32072.nwDV8L.rst @@ -0,0 +1,6 @@ +Fixed issues with binary plists: + +* Fixed saving bytearrays. +* Identical objects will be saved only once. +* Equal references will be load as identical objects. +* Added support for saving and loading recursive data structures.

1 0

bpo-20891: Fix PyGILState_Ensure() (#4650)
by Victor Stinner 30 Nov '17

30 Nov '17

https://github.com/python/cpython/commit/b4d1e1f7c1af6ae33f0e371576c8bcafed… commit: b4d1e1f7c1af6ae33f0e371576c8bcafedb099db branch: master author: Victor Stinner <victor.stinner(a)gmail.com> committer: GitHub <noreply(a)github.com> date: 2017-11-30T22:05:00+01:00 summary: bpo-20891: Fix PyGILState_Ensure() (#4650) When PyGILState_Ensure() is called in a non-Python thread before PyEval_InitThreads(), only call PyEval_InitThreads() after calling PyThreadState_New() to fix a crash. Add an unit test in test_embed. files: A Misc/NEWS.d/next/C API/2017-11-30-18-13-45.bpo-20891.wBnMdF.rst M Doc/c-api/init.rst M Lib/test/test_embed.py M Programs/_testembed.c M Python/pystate.c diff --git a/Doc/c-api/init.rst b/Doc/c-api/init.rst index 2f77bb359d2..a9927aba5e1 100644 --- a/Doc/c-api/init.rst +++ b/Doc/c-api/init.rst @@ -58,8 +58,9 @@ The following functions can be safely called before Python is initialized: The following functions **should not be called** before :c:func:`Py_Initialize`: :c:func:`Py_EncodeLocale`, :c:func:`Py_GetPath`, - :c:func:`Py_GetPrefix`, :c:func:`Py_GetExecPrefix` and - :c:func:`Py_GetProgramFullPath` and :c:func:`Py_GetPythonHome`. + :c:func:`Py_GetPrefix`, :c:func:`Py_GetExecPrefix`, + :c:func:`Py_GetProgramFullPath`, :c:func:`Py_GetPythonHome` and + :c:func:`PyEval_InitThreads`. .. _global-conf-vars: diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py index 8d44543ffd6..c7f45b59acc 100644 --- a/Lib/test/test_embed.py +++ b/Lib/test/test_embed.py @@ -198,6 +198,16 @@ def test_pre_initialization_api(self): self.assertEqual(out, '') self.assertEqual(err, '') + def test_bpo20891(self): + """ + bpo-20891: Calling PyGILState_Ensure in a non-Python thread before + calling PyEval_InitThreads() must not crash. PyGILState_Ensure() must + call PyEval_InitThreads() for us in this case. + """ + out, err = self.run_embedded_interpreter("bpo20891") + self.assertEqual(out, '') + self.assertEqual(err, '') + if __name__ == "__main__": unittest.main() diff --git a/Misc/NEWS.d/next/C API/2017-11-30-18-13-45.bpo-20891.wBnMdF.rst b/Misc/NEWS.d/next/C API/2017-11-30-18-13-45.bpo-20891.wBnMdF.rst new file mode 100644 index 00000000000..e89cf1292af --- /dev/null +++ b/Misc/NEWS.d/next/C API/2017-11-30-18-13-45.bpo-20891.wBnMdF.rst @@ -0,0 +1,3 @@ +Fix PyGILState_Ensure(). When PyGILState_Ensure() is called in a non-Python +thread before PyEval_InitThreads(), only call PyEval_InitThreads() after +calling PyThreadState_New() to fix a crash. diff --git a/Programs/_testembed.c b/Programs/_testembed.c index 21aa76e9de3..a528f3e3aa0 100644 --- a/Programs/_testembed.c +++ b/Programs/_testembed.c @@ -1,4 +1,5 @@ #include <Python.h> +#include "pythread.h" #include <inttypes.h> #include <stdio.h> @@ -147,6 +148,53 @@ static int test_pre_initialization_api(void) return 0; } +static void bpo20891_thread(void *lockp) +{ + PyThread_type_lock lock = *((PyThread_type_lock*)lockp); + + PyGILState_STATE state = PyGILState_Ensure(); + if (!PyGILState_Check()) { + fprintf(stderr, "PyGILState_Check failed!"); + abort(); + } + + PyGILState_Release(state); + + PyThread_release_lock(lock); + + PyThread_exit_thread(); +} + +static int test_bpo20891(void) +{ + /* bpo-20891: Calling PyGILState_Ensure in a non-Python thread before + calling PyEval_InitThreads() must not crash. PyGILState_Ensure() must + call PyEval_InitThreads() for us in this case. */ + PyThread_type_lock lock = PyThread_allocate_lock(); + if (!lock) { + fprintf(stderr, "PyThread_allocate_lock failed!"); + return 1; + } + + _testembed_Py_Initialize(); + + unsigned long thrd = PyThread_start_new_thread(bpo20891_thread, &lock); + if (thrd == PYTHREAD_INVALID_THREAD_ID) { + fprintf(stderr, "PyThread_start_new_thread failed!"); + return 1; + } + PyThread_acquire_lock(lock, WAIT_LOCK); + + Py_BEGIN_ALLOW_THREADS + /* wait until the thread exit */ + PyThread_acquire_lock(lock, WAIT_LOCK); + Py_END_ALLOW_THREADS + + PyThread_free_lock(lock); + + return 0; +} + /* ********************************************************* * List of test cases and the function that implements it. @@ -170,6 +218,7 @@ static struct TestCase TestCases[] = { { "forced_io_encoding", test_forced_io_encoding }, { "repeated_init_and_subinterpreters", test_repeated_init_and_subinterpreters }, { "pre_initialization_api", test_pre_initialization_api }, + { "bpo20891", test_bpo20891 }, { NULL, NULL } }; diff --git a/Python/pystate.c b/Python/pystate.c index 0fb8ed07195..500f9676875 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -922,6 +922,8 @@ PyGILState_Ensure(void) { int current; PyThreadState *tcur; + int need_init_threads = 0; + /* Note that we do not auto-init Python here - apart from potential races with 2 threads auto-initializing, pep-311 spells out other issues. Embedders are expected to have @@ -929,12 +931,10 @@ PyGILState_Ensure(void) */ /* Py_Initialize() hasn't been called! */ assert(_PyRuntime.gilstate.autoInterpreterState); + tcur = (PyThreadState *)PyThread_tss_get(&_PyRuntime.gilstate.autoTSSkey); if (tcur == NULL) { - /* At startup, Python has no concrete GIL. If PyGILState_Ensure() is - called from a new thread for the first time, we need the create the - GIL. */ - PyEval_InitThreads(); + need_init_threads = 1; /* Create a new thread state for this thread */ tcur = PyThreadState_New(_PyRuntime.gilstate.autoInterpreterState); @@ -945,16 +945,28 @@ PyGILState_Ensure(void) tcur->gilstate_counter = 0; current = 0; /* new thread state is never current */ } - else + else { current = PyThreadState_IsCurrent(tcur); - if (current == 0) + } + + if (current == 0) { PyEval_RestoreThread(tcur); + } + /* Update our counter in the thread-state - no need for locks: - tcur will remain valid as we hold the GIL. - the counter is safe as we are the only thread "allowed" to modify this value */ ++tcur->gilstate_counter; + + if (need_init_threads) { + /* At startup, Python has no concrete GIL. If PyGILState_Ensure() is + called from a new thread for the first time, we need the create the + GIL. */ + PyEval_InitThreads(); + } + return current ? PyGILState_LOCKED : PyGILState_UNLOCKED; }

1 0