Issue2711
Created on 2018-10-25.22:45:49 by jeff.allen, last changed 2019-04-04.05:40:27 by jeff.allen.
msg12158 (view) |
Author: Jeff Allen (jeff.allen) |
Date: 2018-10-25.22:45:47 |
|
Tests in test_dict and test_dict_jy fail as follows on versions from Java 9. I'm raising this issue to track the failure, while inserting skips to turn the bots green.
======================================================================
FAIL: test_setdefault_atomic (__main__.PyStringMapTest)
----------------------------------------------------------------------
Traceback (most recent call last):
File "C:\Users\Jeff\Documents\Eclipse-O\jython-jvm9\dist\Lib\test\test_dict.py", line 339, in test_setdefault_atomic
self.assertEqual(hashed1.eq_count + hashed2.eq_count, 1)
AssertionError: 2 != 1
======================================================================
FAIL: test_setdefault_atomic (__main__.DictTest)
----------------------------------------------------------------------
Traceback (most recent call last):
File "C:\Users\Jeff\Documents\Eclipse-O\jython-jvm9\dist\Lib\test\test_dict.py", line 339, in test_setdefault_atomic
self.assertEqual(hashed1.eq_count + hashed2.eq_count, 1)
AssertionError: 2 != 1
|
msg12159 (view) |
Author: Jeff Allen (jeff.allen) |
Date: 2018-10-26.18:18:22 |
|
Skipped on Java 9+ for now in: https://hg.python.org/jython/rev/5aa64ce0b8b8
|
msg12410 (view) |
Author: Jeff Allen (jeff.allen) |
Date: 2019-03-29.08:08:54 |
|
I took a second look at this because new failures turned up during #2649. I conclude that it is not valid to expect this test to pass on Jython because it tests for a particular implementation, not for atomicity per se.
The original CPython bug is https://bugs.python.org/issue13521, based on a discussion where P J Eby reminds us dict.setdefault is atomic only if the __hash__ and __eq__ of all keys are defined in C. This is because the internal logic of testing for the key and adding an entry is atomic under the GIL. If that logic calls out to Python, the GIL is (potentially) released to another thread between Python instructions.
The change introduced at that time works (I think) by finding the proper location for the key (using hash and equality) only once and inserting there if necessary. The test actually uses a __hash__ (guaranteed to collide) and __eq__ that are implemented in Python, and tests that they are called only once.
ConcurrentHashMap, and PyDictionary based on it, fails this test, because it may call __eq__ repeatedly. However, it does this under a lock (on the bucket where the key belongs). It does not matter what the implementation of __eq__ is, no other thread gets to manipulate that bucket. (Bets are off, I suspect, if your __hash__ or __eq__ manipulate the container in the owning thread.)
|
msg12420 (view) |
Author: Jeff Allen (jeff.allen) |
Date: 2019-04-04.05:40:27 |
|
On the logic of the previous note, closing as invalid.
|
|
Date |
User |
Action |
Args |
2019-04-04 05:40:27 | jeff.allen | set | status: open -> closed resolution: invalid messages:
+ msg12420 |
2019-03-29 08:08:54 | jeff.allen | set | keywords:
- test failure causes assignee: jeff.allen messages:
+ msg12410 nosy:
+ zyasoft |
2018-10-26 18:18:22 | jeff.allen | set | messages:
+ msg12159 |
2018-10-25 22:45:49 | jeff.allen | create | |
|