Issue1026

classification
Title: list not totally thread safe
Type: crash Severity: major
Components: Core Versions:
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: zyasoft Nosy List: fwierzbicki, pjenvey, zyasoft
Priority: high Keywords:

Created on 2008-04-19.01:11:19 by pjenvey, last changed 2009-05-07.05:58:07 by zyasoft.

Messages
msg3162 (view) Author: Philip Jenvey (pjenvey) Date: 2008-04-19.01:11:18
I've mentioned before that I thought our list object wasn't thread safe 
at all, but it somewhat is -- many PySequenceList methods are 
synchronized.

Some list operations apparently aren't going through PySequenceList and 
aren't thread safe. One I can reproduce somewhat occurs when I throw a 
bunch of connections/threads at a Pylons hello world app (ab -n 1000 -c 
20)

18:02:35,413 INFO  [paste.httpserver.ThreadPool] Cannot use 
kill_thread_limit as ctypes/killthread is not available
serving on 0.0.0.0:5000 view at http://127.0.0.1:5000
java.lang.IndexOutOfBoundsException: Index must be between 0 and 1, but 
was 5
        at org.python.core.AbstractArray.remove(AbstractArray.java:360)
        at org.python.core.PyObjectList.remove(PyObjectList.java:127)
        at 
org.python.core.PySequenceList.remove(PySequenceList.java:107)
        at org.python.core.PyList.del(PyList.java:100)
        at org.python.core.PyList.list_remove(PyList.java:579)
        at org.python.core.PyList$list_remove_exposer.__call__(Unknown 
Source)
        at org.python.core.PyObject.invoke(PyObject.java:2744)
        at 
paste.httpserver$py.worker_thread_callback$51(/Users/pjenvey/src/python/
pylons-related/Paste/paste/httpserver.py:859)
        at 
paste.httpserver$py.call_function(/Users/pjenvey/src/python/pylons-
related/Paste/paste/httpserver.py)
        at org.python.core.PyTableCode.call(PyTableCode.java:178)
        at org.python.core.PyTableCode.call(PyTableCode.java:399)
        at org.python.core.PyTableCode.call(PyTableCode.java:294)
        at org.python.core.PyFunction.__call__(PyFunction.java:212)
        at org.python.core.PyMethod.__call__(PyMethod.java:101)
        at org.python.core.PyObject._callextra(PyObject.java:395)
        at threading$py.run$40(/Users/pjenvey/src/java/jython-
trunk/dist/Lib/threading.py:200)
        at threading$py.call_function(/Users/pjenvey/src/java/jython-
trunk/dist/Lib/threading.py)
        at org.python.core.PyTableCode.call(PyTableCode.java:178)
        at org.python.core.PyTableCode.call(PyTableCode.java:399)
        at org.python.core.PyTableCode.call(PyTableCode.java:294)
        at org.python.core.PyFunction.__call__(PyFunction.java:212)
        at org.python.core.PyMethod.__call__(PyMethod.java:101)
        at org.python.core.PyObject.__call__(PyObject.java:254)
        at org.python.core.PyObject.invoke(PyObject.java:2731)
        at 
threading$py._Thread__bootstrap$41(/Users/pjenvey/src/java/jython-
trunk/dist/Lib/threading.py:209)
        at threading$py.call_function(/Users/pjenvey/src/java/jython-
trunk/dist/Lib/threading.py)
        at org.python.core.PyTableCode.call(PyTableCode.java:178)
        at org.python.core.PyTableCode.call(PyTableCode.java:399)
        at org.python.core.PyTableCode.call(PyTableCode.java:294)
        at org.python.core.PyFunction.__call__(PyFunction.java:212)
        at org.python.core.PyMethod.__call__(PyMethod.java:101)
        at org.python.core.PyObject.__call__(PyObject.java:244)
        at org.python.core.FunctionThread.run(FunctionThread.java:19)
Exception in thread worker 11:Traceback (most recent call last):
  File "/Users/pjenvey/src/java/jython-trunk/dist/Lib/threading.py", 
line 209, in _Thread__bootstrap
    self.run()
  File "/Users/pjenvey/src/java/jython-trunk/dist/Lib/threading.py", 
line 200, in run
    self._target(*self._args, **self._kwargs)
  File "/Users/pjenvey/src/python/pylons-
related/Paste/paste/httpserver.py", line 859, in worker_thread_callback
    self.idle_workers.remove(thread_id)
java.lang.IndexOutOfBoundsException: 
java.lang.IndexOutOfBoundsException: Index must be between 0 and 1, but 
was 5
msg3163 (view) Author: Philip Jenvey (pjenvey) Date: 2008-04-19.01:11:48
I meant I can reproduce it somewhat easily
msg4538 (view) Author: Jim Baker (zyasoft) Date: 2009-04-19.01:25:56
New backing implementation of list (and tuple) in trunk as of r6245.
This *may* fix the issue, I'm not certain what thread safety this
particular use case requires (since it's of course not well defined in
Python).

Regardless, the call path is now significantly simplified.
msg4607 (view) Author: Philip Jenvey (pjenvey) Date: 2009-04-28.20:00:10
This didn't fix it. We need to make remove thread safe, as well as any 
other basic operations people expect to be thread safe from CPython. The 
ol' effbot FAQ entry is the closest thing to a specification that we 
have here

http://effbot.org/pyfaq/what-kinds-of-global-value-mutation-are-thread-
safe.htm
msg4608 (view) Author: Jim Baker (zyasoft) Date: 2009-04-28.20:43:43
We can readily just make all the publicly visible methods in PyList 
synchronized, that should accomplish what is discussed in the effbot 
FAQ, without the limitations discussed about using the GIL to effect 
this sort of behavior.

One option that we may wish to exploit is to use other backing 
implementations for PyList.list, namely CopyOnWriteArrayList.
msg4609 (view) Author: Jim Baker (zyasoft) Date: 2009-04-28.20:44:13
CopyOnWriteArrayList would be a future option of course...
msg4610 (view) Author: Philip Jenvey (pjenvey) Date: 2009-04-28.20:53:15
Sure on the synchronization. gListAllocatedStatus looked like it should 
be volatile, too

I highly doubt CopyOnWriteArrayList would be appropriate as list 
mutations are pretty frequent. Trying it wouldn't hurt I guess

Also if we're abandoning MergeState we should probably just nuke that 
file.
msg4611 (view) Author: Jim Baker (zyasoft) Date: 2009-04-28.21:41:30
CopyOnWriteArrayList would only be an option as a backing store, for the 
usual scenario where readers are the predominant activity. I don't think 
this really needs to be considered until we have optimized for JDK 7 for 
example.

I'll nuke MergeState, with JDK 7 we get back timsort anyway.
msg4620 (view) Author: Jim Baker (zyasoft) Date: 2009-04-30.06:31:13
Relevant methods are now synchronized as of r6279.

Marking as pending until we have a robust test of expected atomicity of
list ops.
msg4621 (view) Author: Jim Baker (zyasoft) Date: 2009-04-30.06:34:00
Relevant methods are now synchronized as of r6279.

Marking as pending until we have a robust test of expected atomicity of
list ops.

Nuked MergeState in r6281.
msg4651 (view) Author: Jim Baker (zyasoft) Date: 2009-05-07.05:58:01
Closing out, based on pjenvey reporting that it's working for him. We 
should have some good robust tests for this, but they're particularly 
tough for thread safety.
History
Date User Action Args
2009-05-07 05:58:07zyasoftsetstatus: pending -> closed
messages: + msg4651
2009-04-30 06:34:01zyasoftsetmessages: + msg4621
2009-04-30 06:33:27zyasoftsetstatus: open -> pending
messages: + msg4620
2009-04-28 21:41:30zyasoftsetmessages: + msg4611
2009-04-28 20:53:15pjenveysetmessages: + msg4610
2009-04-28 20:44:13zyasoftsetmessages: + msg4609
2009-04-28 20:43:43zyasoftsetmessages: + msg4608
2009-04-28 20:00:11pjenveysetmessages: + msg4607
2009-04-19 01:25:57zyasoftsetmessages: + msg4538
2009-04-04 01:40:14zyasoftsetassignee: zyasoft
resolution: accepted
2009-03-14 02:26:17fwierzbickisetpriority: high
2008-11-18 21:49:48zyasoftsetnosy: + zyasoft
2008-04-20 13:17:59fwierzbickisetnosy: + fwierzbicki
2008-04-19 01:11:49pjenveysetmessages: + msg3163
2008-04-19 01:11:20pjenveycreate