This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Adjust test_concurrent_futures to run more of its tests if multiprocessing.synchronize is missing
Type: enhancement Stage: resolved
Components: Library (Lib), Tests Versions: Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: asheesh, asvetlov, bquinlan, davin, pablogsal, pitrou, python-dev, vstinner, yselivanov, zach.ware
Priority: normal Keywords: patch

Created on 2020-05-19 21:07 by asheesh, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 20239 merged python-dev, 2020-05-19 21:24
Messages (5)
msg369393 - (view) Author: Asheesh Laroia (asheesh) * Date: 2020-05-19 21:07
Some parts of test_concurrent_futures make sense to run even on Python builds where multiprocessing.synchronize is absent. At the moment, test_concurrent_futures.py skips all tests in the file if multiprocessing.synchronize is missing from the Python build. I have a patch to enable more tests, which I'll submit as a GitHub pull request.

The reason I want this patch is while working on CPython on Android, I saw that test_concurrent_futures refused to run its test suite, and I became very afraid that this meant that asyncio on CPython on Android was doomed. In fact, I was afraid for no reason. I want future people porting CPython to be less scared than I was. :)

I discovered that the only part of concurrent.futures that requires multiprocessing.synchronize is concurrent.futures.ProcessPoolExecutor.

concurrent.futures.ProcessPoolExecutor already has a way to check at runtime if it can work on the host system, so this patch leans on that.

This patch fixes a few other tests to skip themselves in the case that ProcessPoolExecutor cannot function.

I tested this by building CPython (with my patch) with adding a configure flag on macOS:

$ ./configure --with-pydebug --with-openssl=$(brew --prefix openssl) ac_cv_posix_semaphores_enabled=no

Why multiprocessing.synchronize is missing
------------------------------------------

multiprocessing.synchronize imports _multiprocessing.SemLock, which is based on a libc function called sem_open(), which implements named semaphores, which allows the creation of lock-esque counters that can be used from multiple processes. multiprocessing.c currently only creates the SemLock property if sem_open() is considered "working" through a check in ./configure; I've short-circuited that check above to be a "no".

On Android, sem_open() exists but simply returns ENOSYS; see its implementation here: https://android.googlesource.com/platform/external/pthreads/+/master/sem_open.c

Android isn't the only platform missing sem_open()

Past work
---------

issue3770 is an older conversation about a related issue. issue3770 has also attracted comments from someone who wants CPython's test suite to run to completion on their platform lacking sem_open() and therefore lacking multiprocessing.synchronize. This patch should solve their problem, I hope.

Future work
-----------

Automated testing: In order to prevent regressions, I'm interested in hosting a buildbot for at least one year, hopefully in perpetuity, on which we call ./configure with ac_cv_posix_semaphores_enabled=no.

I'd like to improve the error message when the user imports multiprocessing.synchronize so it points at some new documentation text that I (or someone) adds to CPython, rather than the issue number. I'm OK taking on the work of writing those docs in a follow-up issue if we merge this.

Thanks
------

Thanks to Zachary Ware for discussing this with me in an CPython contributor office hours time slot before I filed the issue. Thanks to Dr. Russell Keith-Magee, Glyph Lefkowitz, Nathaniel Smith, and Geoffrey Thomas for informal conversations and technical research.
msg369633 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2020-05-22 19:40
Adding multiprocessing and concurrent.futures experts.
msg370674 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-06-03 13:52
> Automated testing: In order to prevent regressions, I'm interested in hosting a buildbot for at least one year, hopefully in perpetuity, on which we call ./configure with ac_cv_posix_semaphores_enabled=no.

Can't you tune an unit test to prevent multiprocessing.synchronize to be imported? For example, sys.modules['multiprocessing.synchronize'] = None ensures that "import multiprocessing.synchronize" fails, even if the module exists and works.

$ python3
Python 3.8.3 (default, May 15 2020, 00:00:00) 
>>> import sys
>>> sys.modules['multiprocessing.synchronize'] = None
>>> import multiprocessing.synchronize
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ModuleNotFoundError: import of multiprocessing.synchronize halted; None in sys.modules

A whole buildbot sounds like an overkill solution to this problem.

On the other hand, multiprocessing tests are already ones of the slowest tests of the test suite *because* they try to test all possible combinations.
msg385717 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-01-26 14:23
Is there any update on this issue? If not, I suggest to close it. It doesn't seem to consider enough people and nobody seems to want to implement it.

See also: https://github.com/python/buildmaster-config/issues/203
msg386613 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-02-08 03:16
New changeset bf2e7e55d7306b1e2fce7dce767e8df5ff42cf1c by Asheesh Laroia in branch 'master':
bpo-40692: Run more test_concurrent_futures tests (GH-20239)
https://github.com/python/cpython/commit/bf2e7e55d7306b1e2fce7dce767e8df5ff42cf1c
History
Date User Action Args
2022-04-11 14:59:31adminsetgithub: 84869
2021-02-08 03:16:20pablogsalsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-02-08 03:16:05pablogsalsetnosy: + pablogsal
messages: + msg386613
2021-01-26 14:23:37vstinnersetmessages: + msg385717
2020-06-03 13:52:31vstinnersetnosy: + vstinner
messages: + msg370674
2020-05-22 19:40:48zach.waresetcomponents: + Library (Lib), Tests, - asyncio
2020-05-22 19:40:16zach.waresetversions: + Python 3.10, - Python 3.9
nosy: + bquinlan, pitrou, davin

messages: + msg369633

components: - Tests
2020-05-19 21:24:50python-devsetkeywords: + patch
nosy: + python-dev

pull_requests: + pull_request19527
stage: patch review
2020-05-19 21:07:44asheeshcreate