Issue24583
Created on 2015-07-07 11:56 by serhiy.storchaka, last changed 2022-04-11 14:58 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| test_set__merge_and_mutate.patch | serhiy.storchaka, 2015-07-07 11:56 | review | ||
| index_to_entry.diff | rhettinger, 2015-07-08 15:54 | review | ||
| intermediary.diff | rhettinger, 2015-07-10 03:24 | review | ||
| set_add_entry_leak.patch | serhiy.storchaka, 2015-07-19 19:20 | review | ||
| set_named_exits.diff | rhettinger, 2015-07-20 05:34 | review | ||
| set_self_contained.diff | rhettinger, 2015-07-20 05:55 | Make the logic self-contained so it can't be called incorrectly. | review | |
| Messages (15) | |||
|---|---|---|---|
| msg246403 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2015-07-07 11:56 | |
When the set is not empty and set.update() argument is set that is modified during merging, the crash is caused. Here is a test that reproduces a crash. Only Python 3.5+ is affected. |
|||
| msg246471 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2015-07-08 20:13 | |
LGTM for 3.5. But 3.6 has other bug. Changeset 637e197be547 looks incorrect to me. key should be increfed before calling PyObject_RichCompareBool() for the same reason as startkey. |
|||
| msg246472 - (view) | Author: Raymond Hettinger (rhettinger) * | Date: 2015-07-08 21:40 | |
Can you produce a test case? Perhaps the incref/decref pair ought to be moved into PyObject_RichCompareBool(). It doesn't make much sense for the callers to do the work. |
|||
| msg246477 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2015-07-09 02:09 | |
The same test is crashed in 3.6 even with index_to_entry.diff. ./python -m test.regrtest -F -m test_merge_and_mutate test_set |
|||
| msg246513 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2015-07-09 17:39 | |
> Perhaps the incref/decref pair ought to be moved into PyObject_RichCompareBool(). This wouldn't help because key can be used after PyObject_RichCompareBool(). |
|||
| msg246572 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2015-07-10 17:43 | |
intermediary.diff LGTM. |
|||
| msg246792 - (view) | Author: Roundup Robot (python-dev) | Date: 2015-07-16 06:54 | |
New changeset 5c3812412b6f by Raymond Hettinger in branch '3.5': Issue #24583: Fix crash when set is mutated while being updated. https://hg.python.org/cpython/rev/5c3812412b6f New changeset 05cb67dab161 by Raymond Hettinger in branch 'default': Issue #24583: Fix crash when set is mutated while being updated. https://hg.python.org/cpython/rev/05cb67dab161 |
|||
| msg246947 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2015-07-19 19:20 | |
5c3812412b6f caused a refleak. $ ./python -m test.regrtest -uall -R 3:3 test_set [1/1] test_set beginning 6 repetitions 123456 ...... test_set leaked [23561, 24961, 23961] references, sum=72483 test_set leaked [785, 787, 787] memory blocks, sum=2359 1 test failed: test_set Proposed patch fixes this. |
|||
| msg246958 - (view) | Author: Raymond Hettinger (rhettinger) * | Date: 2015-07-20 05:12 | |
3.6 only. Correct? |
|||
| msg246961 - (view) | Author: Roundup Robot (python-dev) | Date: 2015-07-20 05:23 | |
New changeset acb5b177dd4e by Raymond Hettinger in branch 'default': Issue #24583: Fix refcount leak. https://hg.python.org/cpython/rev/acb5b177dd4e |
|||
| msg246962 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2015-07-20 05:24 | |
AFAIK 3.5+ (not tested). |
|||
| msg246964 - (view) | Author: Raymond Hettinger (rhettinger) * | Date: 2015-07-20 05:34 | |
Added a patch to neaten it up a bit by naming the exit conditions and avoiding the unnecessary extra incref/decref pair around the resize call. |
|||
| msg246965 - (view) | Author: Raymond Hettinger (rhettinger) * | Date: 2015-07-20 05:55 | |
Added a variant patch that brings the steps together in a more logical manner (single entry point at the top and the named exits at the bottom, brings refcount adjustment logic together in a more coherent way). The "restart" target is done the same way as the "top" target in dictobject.c. Added a comment explaining why the pre-increment is necessary. |
|||
| msg246970 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2015-07-20 08:23 | |
Both variants LGTM. But set_self_contained.diff seems better. I suppose this is 3.6 only. |
|||
| msg246977 - (view) | Author: Roundup Robot (python-dev) | Date: 2015-07-20 11:34 | |
New changeset 3f2c12c0abdb by Raymond Hettinger in branch 'default': Issue #24583: Consolidate previous set object updates into a single function https://hg.python.org/cpython/rev/3f2c12c0abdb |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:58:18 | admin | set | nosy:
+ larry, ned.deily github: 68771 |
| 2015-07-20 11:34:55 | rhettinger | set | status: open -> closed resolution: not a bug stage: patch review -> resolved |
| 2015-07-20 11:34:13 | python-dev | set | messages: + msg246977 |
| 2015-07-20 08:23:50 | serhiy.storchaka | set | messages: + msg246970 |
| 2015-07-20 05:55:57 | rhettinger | set | files:
+ set_self_contained.diff messages: + msg246965 |
| 2015-07-20 05:34:26 | rhettinger | set | files:
+ set_named_exits.diff messages: + msg246964 |
| 2015-07-20 05:24:26 | serhiy.storchaka | set | messages: + msg246962 |
| 2015-07-20 05:23:40 | python-dev | set | messages: + msg246961 |
| 2015-07-20 05:12:59 | rhettinger | set | messages: + msg246958 |
| 2015-07-19 19:20:59 | serhiy.storchaka | set | status: closed -> open files: + set_add_entry_leak.patch messages: + msg246947 resolution: fixed -> (no value) |
| 2015-07-16 06:58:35 | rhettinger | set | status: open -> closed resolution: fixed |
| 2015-07-16 06:54:21 | python-dev | set | nosy:
+ python-dev messages: + msg246792 |
| 2015-07-10 17:43:32 | serhiy.storchaka | set | messages: + msg246572 |
| 2015-07-10 03:24:44 | rhettinger | set | files: + intermediary.diff |
| 2015-07-09 17:39:34 | serhiy.storchaka | set | messages: + msg246513 |
| 2015-07-09 02:09:55 | serhiy.storchaka | set | messages: + msg246477 |
| 2015-07-08 21:42:08 | vstinner | set | title: Crash when source set is changed during merging -> set.update(): Crash when source set is changed during merging |
| 2015-07-08 21:40:38 | rhettinger | set | messages: + msg246472 |
| 2015-07-08 20:13:30 | serhiy.storchaka | set | messages: + msg246471 |
| 2015-07-08 20:13:12 | serhiy.storchaka | set | messages: - msg246469 |
| 2015-07-08 20:11:19 | serhiy.storchaka | set | messages: + msg246469 |
| 2015-07-08 16:17:43 | rhettinger | set | stage: needs patch -> patch review |
| 2015-07-08 15:54:01 | rhettinger | set | files: + index_to_entry.diff |
| 2015-07-07 22:52:47 | rhettinger | set | priority: normal -> high |
| 2015-07-07 22:52:03 | rhettinger | set | assignee: rhettinger |
| 2015-07-07 11:56:10 | serhiy.storchaka | create | |