[issue40489] INCREF/DECREFs around the rich comparison needs tests

[Raymond Hettinger]

Raymond Hettinger  added the comment:

Thank you.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40489] INCREF/DECREFs around the rich comparison needs tests

[Dong-hee Na]

Dong-hee Na  added the comment:


New changeset 785f5e6d674306052bf865677d885c30561985ae by Dong-hee Na in branch 
'master':
bpo-40489: Add test case for dict contain use after free (GH-19906)
https://github.com/python/cpython/commit/785f5e6d674306052bf865677d885c30561985ae


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40489] INCREF/DECREFs around the rich comparison needs tests

[Dong-hee Na]

Change by Dong-hee Na :


--
keywords: +patch
pull_requests: +19221
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/19906

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40489] INCREF/DECREFs around the rich comparison needs tests

[Dong-hee Na]

Dong-hee Na  added the comment:

> Would you like to add a test for this case?

Oh, I mean, is it good to add a test for this case?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40489] INCREF/DECREFs around the rich comparison needs tests

[Dong-hee Na]

Dong-hee Na  added the comment:

Would you like to add a test for this case?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40489] INCREF/DECREFs around the rich comparison needs tests

[Dong-hee Na]

Dong-hee Na  added the comment:

I can crash python interpreter with few lines of code when if we remove those 
codes.



class S(str):

def __eq__(self, other):
d.clear()
return NotImplemented

def __hash__(self):
return hash('test')

d = {S(): 'value'}
'test' in d

--
nosy: +corona10

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40489] INCREF/DECREFs around the rich comparison needs tests

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

See issue1517.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40489] INCREF/DECREFs around the rich comparison needs tests

[Raymond Hettinger]

New submission from Raymond Hettinger :

In Objects/dictobject.c, if I remove the INCREF/DECREF pair around 
PyObject_RichCompareBool(), all the tests still pass:

-   Py_INCREF(startkey);
cmp = PyObject_RichCompareBool(startkey, key, Py_EQ);
-   Py_DECREF(startkey);

It would be nice to have tests demonstrating why it is necessary.  IIRC the 
reason is that an object could have a custom comparison that clears the 
enclosing container and frees the element during the comparison.  

Also, it would be nice if PyObject_RichCompareBool() could be redesigned to not 
fail if a borrowed reference gets deallocated while the function is running.  
The incref and decref occur on a hot path, and it is unfortunate that they 
cause extra writes to objects scattered all over memory, likely causing a lot 
of cache misses in real programs.

--
assignee: serhiy.storchaka
components: Interpreter Core
messages: 367997
nosy: rhettinger, serhiy.storchaka
priority: normal
severity: normal
status: open
title: INCREF/DECREFs around the rich comparison needs tests

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com