<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr">Davin, <br></div><div dir="ltr">I am not familiar with the multiprocessing module, so take the following
with a big grain of salt. I took a look at the PR, then I got an idea of how
 multiprocessing module is organized by reading the doc. Here's some 
food for thought in terms of API reorganization.<br><div class="gmail_quote"><div><br>SharedMemoryManager, SharedMemoryServer<br>---------------------------------------<br><br>It appears to me these are the 2 main public classes, and after reading the doc it seems they really belong to "<a href="https://docs.python.org/3/library/multiprocessing.html#multiprocessing-managers">managers</a>" (multiprocessing.managers namespace). Also:<br>* SharedMemoryManager is a subclass of multiprocessing.managers.SyncManager<br>* SharedMemoryServer is a subclass of multiprocessing.managers.Server<br>shared_memory.py could be renamed to _shared_memory.py and managers.py could import and expose these 2 classes only.</div><div><br><div>Support APIs<br>------------</div><div><br>These
 are objects which seem to be used in support of the 2 classes above, 
but apparently are not meant to be public. As such they could simply 
live in _shared_memory.py and not be exposed:<br><br>- shareable_wrap(): used only in SharedMemoryTracker.wrap()<br>- SharedMemoryTracker: used only by SharedMemoryServer<br>- SharedMemory, WindowsNamedSharedMemory, PosixSharedMemory: used by shareable_wrap() and SharedMemoryTracker<br>- ShareableList: it appears this is not used, but by reading <a href="https://github.com/python/cpython/blob/e0e5065daef36dafe10a46eaa8b7800274d73062/Lib/multiprocessing/managers.py#L1194">here</a> I have a doubt: shouldn't it be register()ed against SharedMemoryManager?<br><br>C extension module<br>------------------<br><br>- ExistentialError, Error - it appears these are not used<br>-
 PermissionsException, ExistentialException - I concur with Ronald 
Oussoren's review: you could simply use PyErr_SetFromErrno() and let the
 original OSError exception bubble up. Same for O_CREAT, O_EXCL, O_CREX,
 O_TRUNC which are already exposed in the os module. I have a couple of 
other minor nitpicks re. the code but I will comment on the PR.</div><div><br></div><div>Compatibility<br>-------------<br></div><div><br></div><div><div>I'm
 not sure if SyncManager and SharedMemoryManager are fully interchangeable so I
 think the doc should clarify this. SyncManager handles a certain set of 
<a href="https://github.com/python/cpython/blob/e0e5065daef36dafe10a46eaa8b7800274d73062/Lib/multiprocessing/managers.py#L1183">types</a>. It appears SharedMemoryManager is supposedly able to do the same except for <a href="https://github.com/applio/cpython/blob/516cf4ac14af257913f46216973c09d58eb259d5/Lib/multiprocessing/shared_memory.py#L227">lists</a>. Is my assumption correct? Also, multiprocessing.Manager() by default returns a SyncManager. If we'll get to a point where SyncManager and SharedMemoryManager are able to handle the same types it'd be good to return SharedMemoryManager as the default, but it's probably safer to leave it for later. Unless they are already there (I don't know) it would be good to have a full set of unit-tests for all the register()ed types and test them against SyncManager and SharedMemoryManager. That would give an idea on the real interchangeability of these 2 classes and would also help writing a comprehensive doc. <br></div></div><div><br></div><div>Hope this helps.</div></div></div><br>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div>Giampaolo - <a href="http://grodola.blogspot.com" target="_blank">http://grodola.blogspot.com</a></div><div><br></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div>