Cannot pickle lock objects

[jakirkham]

Encountering issues trying to pickle lock objects. Not sure if this is something that should be permissible or not. Seems cloudpickle just falls back to pickle in this case. Traceback shown below.

>>> import threading
>>> import cloudpickle
>>> l = threading.Lock()
>>> cloudpickle.pickle.dumps(l)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/zopt/conda2/envs/pickle_test/lib/python2.7/pickle.py", line 1380, in dumps
    Pickler(file, protocol).dump(obj)
  File "/zopt/conda2/envs/pickle_test/lib/python2.7/pickle.py", line 224, in dump
    self.save(obj)
  File "/zopt/conda2/envs/pickle_test/lib/python2.7/pickle.py", line 306, in save
    rv = reduce(self.proto)
  File "/zopt/conda2/envs/pickle_test/lib/python2.7/copy_reg.py", line 70, in _reduce_ex
    raise TypeError, "can't pickle %s objects" % base.__name__
TypeError: can't pickle lock objects

name: pickle_test
channels: !!python/tuple
- !!python/unicode
  'conda-forge'
- !!python/unicode
  'defaults'
dependencies:
- conda-forge::ca-certificates=2017.1.23=0
- conda-forge::cloudpickle=0.2.2=py27_2
- conda-forge::dill=0.2.6=py27_0
- conda-forge::ncurses=5.9=10
- conda-forge::openssl=1.0.2h=3
- conda-forge::python=2.7.12=2
- conda-forge::readline=6.2=0
- conda-forge::sqlite=3.13.0=1
- conda-forge::tk=8.5.19=1
- conda-forge::zlib=1.2.11=0
prefix: /zopt/conda2/envs/pickle_test

[jakirkham]

FWIW tried this with dill version 0.2.6 from conda-forge and did not encounter the same issue (added to the environment.yml above in the edit). All other environment details the same.

>>> import dill
>>> import threading
>>> l = threading.Lock()
>>> dill.dumps(l)
'\x80\x02cdill.dill\n_create_lock\nq\x00\x89\x85q\x01Rq\x02.'

[jakirkham]

An interesting related anecdote is using multiprocessing.Lock instead of threading.Lock. In this case, cloudpickle succeeds, but dill fails. However, dill appears to be erroring with intent as opposed to incidentally. Perhaps that is something cloudpickle should be doing with threading.Lock?

>>> import multiprocessing
>>> import cloudpickle
>>> import dill
>>> l = multiprocessing.Lock()
>>> cloudpickle.dumps(l)
'\x80\x02'
>>> dill.dumps(l)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/zopt/conda2/envs/pickle_test/lib/python2.7/site-packages/dill/dill.py", line 259, in dumps
    dump(obj, file, protocol, byref, fmode, recurse)#, strictio)
  File "/zopt/conda2/envs/pickle_test/lib/python2.7/site-packages/dill/dill.py", line 252, in dump
    pik.dump(obj)
  File "/zopt/conda2/envs/pickle_test/lib/python2.7/pickle.py", line 224, in dump
    self.save(obj)
  File "/zopt/conda2/envs/pickle_test/lib/python2.7/pickle.py", line 306, in save
    rv = reduce(self.proto)
  File "/zopt/conda2/envs/pickle_test/lib/python2.7/multiprocessing/synchronize.py", line 95, in __getstate__
    assert_spawning(self)
  File "/zopt/conda2/envs/pickle_test/lib/python2.7/multiprocessing/forking.py", line 52, in assert_spawning
    ' through inheritance' % type(self).__name__
RuntimeError: Lock objects should only be shared between processes through inheritance

[jakirkham]

cc @mrocklin

[mrocklin]

I'm generally against serializing normal locks. It is a dangerous thing to do without understanding what the lock is trying to protect. That being said, you might want to try dask.utils.SerializableLock.

…

On Wed, Feb 15, 2017 at 3:23 PM, jakirkham ***@***.***> wrote: cc @mrocklin <https://github.com/mrocklin> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#81 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AASszKHcv_q1qliSliwvpy4cmv944Tccks5rc17fgaJpZM4MCLKr> .

[jakirkham]

I'm generally against serializing normal locks. It is a dangerous thing to do without understanding what the lock is trying to protect.

I think I agree. Hence the statement, "not sure if this is something that should be permissible or not". Also the reason for cc-ing you.

That said, I think we could do better here than say failing because pickle failed. Instead we could craft a similar nice message to dill's...

RuntimeError: Lock objects should only be shared between processes through inheritance

That being said, you might want to try dask.utils.SerializableLock.

Yep, that was what I was going to try. 😄

[pitrou]

Should we close this?

[jakirkham]

So we are allowing pickling of multiprocessing.Lock. Does that seem like a good idea? If so, then yes we should close. If not, then we should fix this somehow first.

[pitrou]

So we are allowing pickling of multiprocessing.Lock.

Where is that? I don't see any explicit code for that.

[jakirkham]

Please see the code snippet in the details of this comment for an example of this behavior.

[pitrou]

The pickling is obviously bogus (a 2-byte string cannot hold the necessary information for unpickling):

>>> l
<Lock(owner=None)>
>>> cloudpickle.loads(cloudpickle.dumps(l))
Traceback (most recent call last):
  File "<ipython-input-8-595d1896e403>", line 1, in <module>
    cloudpickle.loads(cloudpickle.dumps(l))
EOFError: Ran out of input

The question is: should we special-case all types for which we want to disallow pickling, such that people get an error message? That might be an endless task...

[jakirkham]

The question is: should we special-case all types for which we want to disallow pickling, such that people get an error message?

I'm confused by this question. pickle explicitly disallows this behavior (see snippet below). The reason we have this bug is we are already doing something extra that pickle is not doing. Not that we need to do something extra.

>>> import pickle
>>> import multiprocessing
>>> l = multiprocessing.Lock()
>>> pickle.dumps(l)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/zopt/conda2/envs/pickle_test/lib/python2.7/pickle.py", line 1380, in dumps
    Pickler(file, protocol).dump(obj)
  File "/zopt/conda2/envs/pickle_test/lib/python2.7/pickle.py", line 224, in dump
    self.save(obj)
  File "/zopt/conda2/envs/pickle_test/lib/python2.7/pickle.py", line 306, in save
    rv = reduce(self.proto)
  File "/zopt/conda2/envs/pickle_test/lib/python2.7/copy_reg.py", line 84, in _reduce_ex
    dict = getstate()
  File "/zopt/conda2/envs/pickle_test/lib/python2.7/multiprocessing/synchronize.py", line 95, in __getstate__
    assert_spawning(self)
  File "/zopt/conda2/envs/pickle_test/lib/python2.7/multiprocessing/forking.py", line 52, in assert_spawning
    ' through inheritance' % type(self).__name__
RuntimeError: Lock objects should only be shared between processes through inheritance

[pitrou]

I'm wondering what makes cloudpickle not call __getstate__ here?

[pierreglaser]

An interesting related anecdote is using multiprocessing.Lock instead of threading.Lock. In this case, cloudpickle succeeds, but dill fails

Launching these commands on Python 3.6:

Python 3.6.12 |Anaconda, Inc.| (default, Sep  8 2020, 23:10:56)
Type 'copyright', 'credits' or 'license' for more information
IPython 7.16.1 -- An enhanced Interactive Python. Type '?' for help.

In [1]: import cloudpickle

In [2]: import multiprocessing

In [3]: cloudpickle.dumps(multiprocessing.Lock())
---------------------------------------------------------------------------
RuntimeError                              Traceback (most recent call last)
<ipython-input-3-5671a97d518f> in <module>
----> 1 cloudpickle.dumps(multiprocessing.Lock())

~/.local/miniconda3/envs/py36/lib/python3.6/site-packages/cloudpickle/cloudpickle_fast.py in dumps(obj, protocol)
    100         with io.BytesIO() as file:
    101             cp = CloudPickler(file, protocol=protocol)
--> 102             cp.dump(obj)
    103             return file.getvalue()
    104

~/.local/miniconda3/envs/py36/lib/python3.6/site-packages/cloudpickle/cloudpickle_fast.py in dump(self, obj)
    561     def dump(self, obj):
    562         try:
--> 563             return Pickler.dump(self, obj)
    564         except RuntimeError as e:
    565             if "recursion" in e.args[0]:

~/.local/miniconda3/envs/py36/lib/python3.6/pickle.py in dump(self, obj)
    407         if self.proto >= 4:
    408             self.framer.start_framing()
--> 409         self.save(obj)
    410         self.write(STOP)
    411         self.framer.end_framing()

~/.local/miniconda3/envs/py36/lib/python3.6/pickle.py in save(self, obj, save_persistent_id)
    494             reduce = getattr(obj, "__reduce_ex__", None)
    495             if reduce is not None:
--> 496                 rv = reduce(self.proto)
    497             else:
    498                 reduce = getattr(obj, "__reduce__", None)

~/.local/miniconda3/envs/py36/lib/python3.6/multiprocessing/synchronize.py in __getstate__(self)
     99
    100     def __getstate__(self):
--> 101         context.assert_spawning(self)
    102         sl = self._semlock
    103         if sys.platform == 'win32':

~/.local/miniconda3/envs/py36/lib/python3.6/multiprocessing/context.py in assert_spawning(obj)
    354         raise RuntimeError(
    355             '%s objects should only be shared between processes'
--> 356             ' through inheritance' % type(obj).__name__
    357             )

RuntimeError: Lock objects should only be shared between processes through inheritance

[jakirkham]

Maybe this got fixed at some point then?

Edit: Do we have a test covering that?

[jakirkham]

James just added PR ( #449 ) with a test cover this case. Have gone ahead and merged since it seems like we are ok with the current behavior. Though please let us know if that is not the case