Fix race condition in PyStringMap keys method

[tpoliaw]

If the map was modified during a call to keys(), the keyset would change
size and cause an ArrayIndexOutOfBoundsException. This creates a copy of
the keyset and uses that throughout the method.

Sample to show bug in previous version

from threading import Thread

def update_globals():
    for i in range(1000):
        globals()['thing%d'%i] = i

t = Thread(target=update_globals)
t.start()
for i in range(1000):
    dir()

Would result in (with varying index each time)

Traceback (most recent call last):
  File "/tmp/demo.py", line 10, in <module>
    dir()
java.lang.ArrayIndexOutOfBoundsException: 22
	at org.python.core.PyStringMap.stringmap_keys(PyStringMap.java:643)
	at org.python.core.PyStringMap$stringmap_keys_exposer.__call__(Unknown Source)
	at org.python.core.PyObject.invoke(PyObject.java:3634)
	at org.python.core.__builtin__.dir(__builtin__.java:457)
	at org.python.core.BuiltinFunctions.__call__(__builtin__.java:40)
	at org.python.core.PyObject.__call__(PyObject.java:450)
	at org.python.pycode._pyx0.f$0(/tmp/demo.py:9)
	at org.python.pycode._pyx0.call_function(/tmp/demo.py)
	at org.python.core.PyTableCode.call(PyTableCode.java:173)
	at org.python.core.PyCode.call(PyCode.java:18)
	at org.python.core.Py.runCode(Py.java:1687)
	at org.python.util.PythonInterpreter.execfile(PythonInterpreter.java:299)
	at org.python.util.jython.runSimpleFile(jython.java:363)
	at org.python.util.jython.runStream(jython.java:395)
	at org.python.util.jython.runFile(jython.java:420)
	at org.python.util.jython.main(jython.java:615)
java.lang.ArrayIndexOutOfBoundsException: java.lang.ArrayIndexOutOfBoundsException: 22

I can add this as a regression test but wasn't sure what the best approach was for tests that might have different results each time it's run.

[jeff5]

Thanks for looking into this and a potential solution.

The thread synchronisation implicit in some operations in CPython, where they happen to be implemented in C, sets up expectations I sometimes think unreasonable.

This seems simple enough that I wouldn't press for a regression test. I think it needs a short comment that we have to take a stable local copy of the keys because they may change while iterating (which is not an error).

Looking deeper, the use of ConcurrentHashMap's iterator inside toArray()) is supplying that tolerance for concurrent change.

[jeff5]

@tpoliaw : I'd be happy to incorporate this if you sign the CLA. It involves creating an account at bugs.python.org (a chore), and telling it your GitHub name, but on the plus side, then you're set up to contribute to CPython too.

[tpoliaw]

I think I signed the CLA a while ago with this account's email but will add my github name to it.

[jeff5]

That checks out now, thanks. Sorry I didn't recognise that you'd contributed before.

[jeff5]

Now in at https://hg.python.org/jython/rev/57e47c817d26 . Thanks Peter.

[jeff5]

Raised as jythontools/jython pull 156.