Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

get_lock() method is not present for Values created using multiprocessing.Manager() #79967

Open
LorenzoPersichetti mannequin opened this issue Jan 19, 2019 · 14 comments
Open
Labels
3.7 (EOL) end of life docs Documentation in the Doc dir stdlib Python modules in the Lib dir topic-multiprocessing

Comments

@LorenzoPersichetti
Copy link
Mannequin

LorenzoPersichetti mannequin commented Jan 19, 2019

BPO 35786
Nosy @pfmoore, @pitrou, @tjguk, @zware, @MojoVampire, @applio, @websurfer5, @ian-bertolacci

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2019-01-19.21:50:32.910>
labels = ['3.7', 'library', 'docs']
title = 'get_lock() method is not present for Values created using multiprocessing.Manager()'
updated_at = <Date 2020-08-15.01:55:10.127>
user = 'https://bugs.python.org/LorenzoPersichetti'

bugs.python.org fields:

activity = <Date 2020-08-15.01:55:10.127>
actor = 'ned.deily'
assignee = 'docs@python'
closed = False
closed_date = None
closer = None
components = ['Documentation', 'Library (Lib)']
creation = <Date 2019-01-19.21:50:32.910>
creator = 'Lorenzo Persichetti'
dependencies = []
files = []
hgrepos = []
issue_num = 35786
keywords = []
message_count = 6.0
messages = ['334070', '334106', '349155', '349156', '349191', '373174']
nosy_count = 11.0
nosy_names = ['paul.moore', 'pitrou', 'tim.golden', 'docs@python', 'zach.ware', 'nemeskeyd', 'josh.r', 'davin', 'Lorenzo Persichetti', 'Jeffrey.Kintscher', 'IanBertolacci']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue35786'
versions = ['Python 3.6', 'Python 3.7']

@LorenzoPersichetti
Copy link
Mannequin Author

LorenzoPersichetti mannequin commented Jan 19, 2019

According to the documentation of the multiprocessing.Value() class available here https://docs.python.org/3.6/library/multiprocessing.html#multiprocessing.Value

Operations like += which involve a read and write are not atomic. So if, for instance, you want to atomically increment a shared value it is insufficient to just do

counter.value += 1
Assuming the associated lock is recursive (which it is by default) you can instead do

with counter.get_lock():
    counter.value += 1

What happens is that when running the following snippet

import multiprocessing
manager = multiprocessing.Manager()
value = manager.Value('i', 0)
value.get_lock()

the result is
AttributeError: 'ValueProxy' object has no attribute 'get_lock'

@LorenzoPersichetti LorenzoPersichetti mannequin added the OS-windows label Jan 19, 2019
@LorenzoPersichetti LorenzoPersichetti mannequin added stdlib Python modules in the Lib dir docs Documentation in the Doc dir labels Jan 19, 2019
@pablogsal
Copy link
Member

You are not using multiprocessing.Value:

>>> import multiprocessing
>>> x = multiprocessing.Value("i", 0)
>>> x.get_lock()
<RLock(None, 0)>

@nemeskeyd
Copy link
Mannequin

nemeskeyd mannequin commented Aug 7, 2019

Nothing in the documentation says that multiprocessing.Value and the object returned by manager.Value() is any different. Nor is it clear why they should be.

It is perfectly understandable to expect that manager.Value() is actually of type multiprocessing.Value. It is also a valid assumption that the "canonical" way to acquire such objects are from a Manager, not directly. An example that reinforces this assumption is that of the Queue class, which HAS to be created through a Manager, lest we get "RuntimeError: Queue objects should only be shared between processes through inheritance".

In conclusion, I think this is definitely a valid issue. What I am not so sure about is if it is (just) an issue in the documentation or the API itself.

@nemeskeyd nemeskeyd mannequin added the 3.7 (EOL) end of life label Aug 7, 2019
@nemeskeyd
Copy link
Mannequin

nemeskeyd mannequin commented Aug 7, 2019

OK, actually: trying to create a multiprocessing.Value object and sharing it between a Pool of processes results in "RuntimeError: Synchronized objects should only be shared between processes through inheritance". So the only way seems to be through a Manager, but its Value() is of a different class?

@MojoVampire
Copy link
Mannequin

MojoVampire mannequin commented Aug 7, 2019

Reading the docs, I'd definitely expect multiprocessing.Manager().Value to obey the same interface as multiprocessing.Value. The SyncManager docs say:

Its methods create and return Proxy Objects for a number of commonly used data types to be synchronized across processes.

It does also say (under the linked definition for Proxy Objects):

A proxy object has methods which invoke corresponding methods of its referent (although not every method of the referent will necessarily be available through the proxy).

which implies the proxy might be more limited, but in fact the proxy is wrapping a completely different underlying class, multiprocessing.managers.Value (that has nothing to do with the class you'd get from calling multiprocessing.Value or multiprocessing.sharedctypes.Value).

It looks like there was, at some point, an attempt to make certain interfaces match; multiprocessing.managers.Value's initializer has a prototype of:

def __init__(self, typecode, value, lock=True):

(documented prototype excludes the lock argument), but AFAICT, lock is completely ignored, and typecode is stored as _typecode on the underlying Value, but otherwise ignored; the multiprocessing.managers.ValueProxy object returned by manager.Value() doesn't expose it (except insofar as you can call ._getvalue() on it to retrieve a copy of the underlying Value), and even if you get the unwrapped object, all _typecode does is change the repr; it's not used anywhere else.

It seems like SyncManager.Value is just the worst of all possible worlds; it has a prototype that (roughly) matches multiprocessing.Value/multiprocessing.sharedctypes.Value, and the limited documentation implies it serves the same function, but:

  1. It effectively ignores every argument aside from value (which is the second argument, so you're stuck passing the first argument even though nothing uses it)
  2. It doesn't actually use ctypes to achieve efficient storage/IPC communication (updating value involves pickling it, not just sending the raw data of a C array/struct)
  3. It doesn't provide any of the interface of the other Value, so you can't use get_lock (or use a with statement to lock it, or manually call acquire or release) or get_obj.

I'm not sure what to do about it though; the manager version of Value is much more flexible, and I'm sure there is existing code that takes advantage of that, so we can't rewrite to make it ctypes backed. The multiprocessing.managers code is too new/complex for me to determine if any easy solution exists to expand multiprocessing.managers.ValueProxy to support context management/get_lock/get_obj to match the behavior of the class returned by multiprocessing.Value/multiprocessing.sharedctypes.Value, but it seems like, if we're going to let the docs imply a relationship, we ought to at least try to make the API of the two classes roughly match.

@ian-bertolacci
Copy link
Mannequin

ian-bertolacci mannequin commented Jul 6, 2020

What's being done about this?
I would say this is less "misleading documentation" and more "incorrect implementation"

There is also not an obvious temporary work-around.

@zooba zooba removed the OS-windows label Jul 6, 2020
@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@xintenseapple
Copy link

Are there any proposed solutions to this problem or any workaround aside from using an additional SyncManager.Lock object to synchronize access to a SyncManager.Value?

@rojikada
Copy link

rojikada commented Dec 9, 2022

This issue is still present since 2019, also just encountered this. I guess I will have to stick with additional Lock, but I welcome any updates on this.

@fearedbliss
Copy link

I just ran into this as well (expected manager.Value to be the same as multiprocessing.Value)

@lrlunin
Copy link

lrlunin commented Jan 30, 2023

I've just encountered the same problem. I guess the main issue that the description of ValueProxy is missing in the official documentation. There is one variable value and two methods, namely set()/get(), available. I might guess that the value field of ValueProxy is identical to Value's value field and is not thread-safe. I also guess that the set()/get() methods use some kind of locks internally and might be thread-safe interface for reading and writing to the variable.

It's nothing but my assumption and guess. I would ask the maintainers to describe this class in the documentation. Because right now there is even no mention of the ValueProxy in the docs.

@TheMatt2
Copy link

TheMatt2 commented Mar 5, 2023

Ran into this as well. Surprised to find such a sharp corner in multiprocessing. Since concurrent.futures.ProcessPoolExecutor requires multiprocessing.Manager() objects, I was not able to use the normal value objects.
I would not have expected the current behavior based on the documentation.

Experienced the issue in Python 3.9

@arrmansa
Copy link

arrmansa commented Feb 22, 2024

Just ran into this as well while trying to increment a manager.Value counter in concurrent.futures.ProcessPoolExecutor. If we can't change the existing manager.Value can we have a different attribute that is the same as multiprocessing.Value ? maybe manager.cValue or something else?

@RSKothari
Copy link

How is this issue still open? I am facing the same problem in Python 3.11.8.

    list(tqdm.tqdm(pool.starmap(process_event_file, [(event_file, path_figures, count_saccades, count_fixations, count_blinks) for event_file in event_files]), total=len(event_files)))
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/anaconda3/lib/python3.11/multiprocessing/pool.py", line 375, in starmap
    return self._map_async(func, iterable, starmapstar, chunksize).get()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/anaconda3/lib/python3.11/multiprocessing/pool.py", line 774, in get
    raise self._value
AttributeError: 'ValueProxy' object has no attribute 'get_lock'

@james-sullivan
Copy link

james-sullivan commented Nov 21, 2024

I ran into the same issue with Python 3.12.4

Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/multiprocessing/process.py", line 314, in _bootstrap
    self.run()
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/multiprocessing/process.py", line 108, in run
    self._target(*self._args, **self._kwargs)
  File "/Users/james/dev/PythonConcurrency/producer_consumer_practice.py", line 20, in produce
    with finished_producers.get_lock():
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'ValueProxy' object has no attribute 'get_lock'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.7 (EOL) end of life docs Documentation in the Doc dir stdlib Python modules in the Lib dir topic-multiprocessing
Projects
Status: No status
Development

No branches or pull requests