classification
Title: Crash in insertdict
Type: crash Stage: resolved
Components: Interpreter Core Versions: Python 3.5, Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, Mark.Shannon, georg.brandl, pitrou, python-dev, vstinner
Priority: normal Keywords: patch

Created on 2014-10-16 12:21 by pitrou, last changed 2014-10-17 22:38 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
randomsecret.patch pitrou, 2014-10-16 12:31 review
dictcrash.py pitrou, 2014-10-16 12:31
insertdict.patch pitrou, 2014-10-16 13:52 review
Messages (12)
msg229524 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-10-16 12:21
I got a weird crash in an interpreter session. Here is what I did:

$ ./python 
Python 3.5.0a0 (default:fd658692db3a+, Oct 15 2014, 23:13:43) 
[GCC 4.8.1] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> f = open('toto', 'ab')
>>> f.write(b'bb')
2
>>> f.flush()
>>> 
>>> f = open('toto', 'ab')
__main__:1: ResourceWarning: unclosed file <_io.BufferedWriter name='toto'>
python: Objects/dictobject.c:855: insertdict: Assertion `ep->me_key != ((void *)0) && ep->me_key != (&_dummy_struct)' failed.
Abandon (core dumped)


Here are the inner frames of the traceback:

(gdb) bt
#0  0x00007f27e691df77 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  0x00007f27e69215e8 in __GI_abort () at abort.c:90
#2  0x00007f27e6916d43 in __assert_fail_base (fmt=0x7f27e6a6df58 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", 
    assertion=assertion@entry=0x6878d0 "ep->me_key != ((void *)0) && ep->me_key != (&_dummy_struct)", file=file@entry=0x6874f2 "Objects/dictobject.c", 
    line=line@entry=855, function=function@entry=0x6880a0 <__PRETTY_FUNCTION__.10152> "insertdict") at assert.c:92
#3  0x00007f27e6916df2 in __GI___assert_fail (assertion=0x6878d0 "ep->me_key != ((void *)0) && ep->me_key != (&_dummy_struct)", 
    file=0x6874f2 "Objects/dictobject.c", line=855, function=0x6880a0 <__PRETTY_FUNCTION__.10152> "insertdict") at assert.c:101
#4  0x00000000004b65d0 in insertdict (mp=0x7f27e76f9838, key='f', hash=-9123380860235530973, value=<_io.BufferedWriter at remote 0x7f27e766e758>)
    at Objects/dictobject.c:855
#5  0x00000000004b752a in PyDict_SetItem (
    op={'f': <_io.BufferedWriter at remote 0x7f27e766e758>, '__builtins__': <module at remote 0x7f27e7750358>, '__spec__': None, '__warningregistry__': {'version': 0}, '__doc__': None, 'rlcompleter': <module at remote 0x7f27e5f0b358>, '__name__': '__main__', '__cached__': None, '__package__': None, '__loader__': <SourceFileLoader(name='__main__', path='/home/antoine/.pythonrc.py') at remote 0x7f27e765d468>, 'readline': <module at remote 0x7f27e5f0b058>}, 
    key='f', value=<_io.BufferedWriter at remote 0x7f27e766e758>) at Objects/dictobject.c:1245
#6  0x00000000005a9f7c in PyEval_EvalFrameEx (f=Frame 0x7f27e7704d78, for file <stdin>, line 1, in <module> (), throwflag=0) at Python/ceval.c:2065


Here are the hash initialization values:

(gdb) p _Py_HashSecret
$1 = {uc = "\260\306\vA\a\342\274\337\341\026\003\bbq\366\f\"\032E\232jb%\023", fnv = {prefix = -2324734786846079312, suffix = 934058638581110497}, 
  siphash = {k0 = 16122009286863472304, k1 = 934058638581110497}, djbx33a = {padding = "\260\306\vA\a\342\274\337\341\026\003\bbq\366\f", 
    suffix = 1379617070853200418}, expat = {padding = "\260\306\vA\a\342\274\337\341\026\003\bbq\366\f", hashsalt = 1379617070853200418}}
(gdb) p PyHash_Func
$2 = {hash = 0x5ee557 <siphash24>, name = 0x6b2020 "siphash24", hash_bits = 64, seed_bits = 128}


The crash seems difficult to reproduce simply by typing the lines above. Perhaps by forcing the hash initialization values as above.
msg229525 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-10-16 12:31
Ok, here is how to reproduce deterministically:

1) apply randomsecret.patch to hardcode the siphash initialization values
2) run "cat dictcrash.py | ./python -i":

Python 3.5.0a0 (default:030fda7b1de8+, Oct 16 2014, 14:27:32) 
[GCC 4.8.1] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> >>> >>> >>> __main__:1: ResourceWarning: unclosed file <_io.BufferedWriter name='x'>
python: Objects/dictobject.c:855: insertdict: Assertion `ep->me_key != ((void *)0) && ep->me_key != (&_dummy_struct)' failed.
Abandon
msg229526 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-10-16 12:45
Looking down into the insertdict frame:

(gdb) p key
$14 = 'f'
(gdb) p old_value
$15 = <unknown at remote 0x7f27e766e678>
(gdb) p *ep
$16 = {me_hash = -3761688987579986997, me_key = 0x0, me_value = 0x0}


So this seems to have hit the following line in insertdict():

    if (old_value != NULL) {
        assert(ep->me_key != NULL && ep->me_key != dummy);
        *value_addr = value;
--->    Py_DECREF(old_value); /* which **CAN** re-enter */
    }

And it *has* reentered because the ResourceWarning inserted the __warningregistry__ attribute into the __main__ dict.

Note the reentrancy looks safe: the dict should be in a stable state at this point (?). But the assert at the end of insertdict assumes the dict hasn't mutated... Why?
msg229528 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-10-16 13:27
So after moving away that specific assert, there doesn't seem to be any problem (I ran the whole test suite in the same interpreter).

If it's really a case of the assert being too strict, then the issue isn't very severe, as most people would you a non-debug build.
msg229529 - (view) Author: Mark Shannon (Mark.Shannon) * Date: 2014-10-16 13:43
The assertion on line 855
is a duplicate of the assertion on line 821, which is just before the reentrant DECREF. 
The assertion on line 821 appears to be correct.

I don't know why the assertion is at the end of the function rather than earlier, but hg blame says its my code :(

I'll put a patch together this weekend, once I've had time to reproduce the failure.
msg229530 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-10-16 13:52
Here is a patch, with a test that fails deterministically without the fix.
msg229531 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-10-16 13:58
Le 16/10/2014 15:43, Mark Shannon a écrit :
> 
> I'll put a patch together this weekend, once I've had time to reproduce the failure.

Sorry, I was already writing mine when you said that :-)
msg229539 - (view) Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * Date: 2014-10-16 17:04
Crash when running test_reentrant_insertion is reproducible also in 3.3 branch, but not in 3.2 branch.
insertdict.patch applies and works in 3.3 branch.
msg229540 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-10-16 17:05
But 3.3 only receives security fixes, and this is not a security issue. It's up to Georg whether we really wants to backport the fix.
msg229569 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2014-10-17 08:57
This is not a crash, but an abort, so it's not a security issue.
msg229614 - (view) Author: Roundup Robot (python-dev) Date: 2014-10-17 22:35
New changeset 9ec84f9b61c6 by Antoine Pitrou in branch '3.4':
Issue #22653: Fix an assertion failure in debug mode when doing a reentrant dict insertion in debug mode.
https://hg.python.org/cpython/rev/9ec84f9b61c6

New changeset 4ff865976bb9 by Antoine Pitrou in branch 'default':
Issue #22653: Fix an assertion failure in debug mode when doing a reentrant dict insertion in debug mode.
https://hg.python.org/cpython/rev/4ff865976bb9
msg229615 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-10-17 22:38
Ok, then I've committed the patch to 3.4 and 3.5.
History
Date User Action Args
2014-10-17 22:38:27pitrousetstatus: open -> closed
resolution: fixed
messages: + msg229615

stage: patch review -> resolved
2014-10-17 22:35:54python-devsetnosy: + python-dev
messages: + msg229614
2014-10-17 22:32:51pitrousetversions: - Python 3.3
2014-10-17 08:57:50georg.brandlsetmessages: + msg229569
2014-10-16 17:05:46pitrousetnosy: + georg.brandl
messages: + msg229540
2014-10-16 17:04:33Arfreversetmessages: + msg229539
versions: + Python 3.3
2014-10-16 13:58:44pitrousetmessages: + msg229531
title: Crash in insertdict in debug mode -> Crash in insertdict
2014-10-16 13:52:39pitrousetfiles: + insertdict.patch
priority: high -> normal
title: Crash in insertdict -> Crash in insertdict in debug mode
messages: + msg229530

stage: patch review
2014-10-16 13:43:09Mark.Shannonsetmessages: + msg229529
2014-10-16 13:27:28pitrousetmessages: + msg229528
2014-10-16 12:45:37pitrousetmessages: + msg229526
2014-10-16 12:33:22pitrousetpriority: normal -> high
versions: + Python 3.4
2014-10-16 12:31:34pitrousetfiles: + dictcrash.py
2014-10-16 12:31:22pitrousetfiles: + randomsecret.patch
keywords: + patch
messages: + msg229525
2014-10-16 12:29:27vstinnersetnosy: + vstinner
2014-10-16 12:23:30Arfreversetnosy: + Arfrever
2014-10-16 12:21:14pitroucreate