bpo-36473: add maximum iteration check for dict .values() and .items()

[thp]

No description provided.

[thp]

I see the tests failing because the memory size for everything that uses dict keys changes with that (obviously). Any input on if/how/when this ABI change is possible/worth it would be greatly appreciated (also if it's just enough to adjust the sizes in the test case).

[thp]

To summarize the "cost" of handling these corner cases:

1.) 1x ssize_t (8 bytes on 64 bit, 4 bytes on 32 bit) additional memory overhead for each dict
2.) 1x increment at each dict keys change (of the revision variable)
3.) 1x comparison during each dict iteration

Alternatively, I could prepare a PR that applies #12596 to dictiter_iternextvalue() and dictiter_iternextitem(), not fixing the remaining corner case, but at least having no additional memory cost and only paying for one length comparison (di->len == 0) for each key iteration.

Could this still be a case that's enabled in Py_DEBUG builds to detect usage of undefined behavior?

[thp]

An alternative; without memory overhead, only tracking the maximum iteration count:

https://github.com/thp/cpython/commit/ace18d85bc32fa9e5dcb17b442c72c954d9367cf

[methane]

An alternative; without memory overhead, only tracking the maximum iteration count:

Please do this.

Not raising RuntimeException is not a bug. I don't want to add member only for it.

[thp]

@methane I re-used this pull request (hope this is okay) and updated the changes now to only add the maximum iteration checks to .values() and .items().

[thp]

There seem to be some (temporary?) test failures in Azure Pipelines unrelated to the change, it is possible to trigger a re-build?

(Edit: Force-pushed the amended commit now to trigger a rebuild.)

======================================================================
FAIL: test_unwritable_directory (test.test_import.PycacheTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vsts/work/1/s/Lib/test/test_import/__init__.py", line 822, in test_unwritable_directory
    self.assertFalse(os.path.exists(pyc_path),
AssertionError: True is not false : bytecode file '__pycache__/@test_838_tmp.cpython-38.pyc' for '@test_838_tmp' exists

...

======================================================================
FAIL: test_open_mode (test.test_pathlib.PosixPathTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vsts/work/1/s/Lib/test/test_pathlib.py", line 2109, in test_open_mode
    self.assertEqual(stat.S_IMODE(st.st_mode), 0o644)
AssertionError: 438 != 420

...

======================================================================
FAIL: test_touch_mode (test.test_pathlib.PosixPathTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vsts/work/1/s/Lib/test/test_pathlib.py", line 2121, in test_touch_mode
    self.assertEqual(stat.S_IMODE(st.st_mode), 0o644)
AssertionError: 438 != 420

...

======================================================================
FAIL: test_file_mode (test.test_tarfile.Bz2StreamWriteTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vsts/work/1/s/Lib/test/test_tarfile.py", line 1420, in test_file_mode
    self.assertEqual(mode, 0o644, "wrong file permissions")
AssertionError: 438 != 420 : wrong file permissions

...

======================================================================
FAIL: test_file_mode (test.test_tarfile.GzipStreamWriteTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vsts/work/1/s/Lib/test/test_tarfile.py", line 1420, in test_file_mode
    self.assertEqual(mode, 0o644, "wrong file permissions")
AssertionError: 438 != 420 : wrong file permissions

...

======================================================================
FAIL: test_file_mode (test.test_tarfile.LzmaStreamWriteTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vsts/work/1/s/Lib/test/test_tarfile.py", line 1420, in test_file_mode
    self.assertEqual(mode, 0o644, "wrong file permissions")
AssertionError: 438 != 420 : wrong file permissions

...

======================================================================
FAIL: test_file_mode (test.test_tarfile.StreamWriteTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vsts/work/1/s/Lib/test/test_tarfile.py", line 1420, in test_file_mode
    self.assertEqual(mode, 0o644, "wrong file permissions")
AssertionError: 438 != 420 : wrong file permissions

...

======================================================================
FAIL: test_mode (test.test_os.MakedirTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vsts/work/1/s/Lib/test/test_os.py", line 1157, in test_mode
    self.assertEqual(os.stat(parent).st_mode & 0o777, 0o775)
AssertionError: 511 != 509

[methane]

Please update Misc/NEWS.d/next/Core and Builtins/2019-03-27-23-53-00.bpo-36452.xhK2lT.rst instead of adding new entry.
News entry is not only for credit, but also Python users.

[thp]

Done + PR updated.