Skip to content

Commit 72c74e7

Browse files
committed
Fix bug and refactor code
- Fix bug: the lost of errno value in SemLock_impl, - Reduce shared memory sise, used kern.posix.max.sem value instaed of SC_SEM_NSEMS_MAX value for array size of shared memeory, - Add a unit test that raises an error when the maximum opened sems is reached. Last code reviews are relaized with Claude code
1 parent d870e18 commit 72c74e7

5 files changed

Lines changed: 54 additions & 14 deletions

File tree

Lib/test/_test_multiprocessing.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7545,7 +7545,8 @@ def test_sharedmem_details(self):
75457545
with memoryview(_mmap[:16]).cast('i') as _buf: # signed int
75467546
header = _buf[:4].tolist()
75477547
self.assertEqual(header[0], n)
7548-
self.assertEqual(header[-2], size)
7548+
self.assertEqual(header[1], _multiprocessing._MACOSX_MAX_OPEN_SEMS)
7549+
self.assertEqual(header[2], size)
75497550
sizeof_counter = header[-1]
75507551

75517552
start += _len
@@ -7597,6 +7598,12 @@ def test_sharedmem_details(self):
75977598
while len(sems) and (s := sems.pop()):
75987599
del s
75997600

7601+
def test_sharedmem_max_open_sems(self):
7602+
# This test should be run alone.
7603+
with self.assertRaisesRegex(OSError, "No space left on device"):
7604+
sems = []
7605+
for i in range(1, _multiprocessing._MACOSX_MAX_OPEN_SEMS + 1):
7606+
sems.append(self.Semaphore(i) if i % 2 else self.BoundedSemaphore(i))
76007607

76017608
#
76027609
# Mixins

Modules/_multiprocessing/multiprocessing.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ multiprocessing_exec(PyObject *module)
235235
Py_DECREF(py_sem_value_max);
236236

237237
#ifdef HAVE_BROKEN_SEM_GETVALUE
238-
if (_PyMp_init_shm_names(module) < 0)
238+
if (_PyMp_init_module_constants(module) < 0)
239239
return -1;
240240
#endif
241241

Modules/_multiprocessing/multiprocessing.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ PyObject *_PyMp_SetError(PyObject *Type, int num);
101101
extern PyType_Spec _PyMp_SemLockType_spec;
102102
extern PyObject *_PyMp_sem_unlink(const char *name);
103103
#ifdef HAVE_BROKEN_SEM_GETVALUE
104-
extern int _PyMp_init_shm_names(PyObject *module);
104+
extern int _PyMp_init_module_constants(PyObject *module);
105105
extern PyObject *_multiprocessing_set_shm_names(PyObject *module, PyObject *args);
106106
#endif
107107

Modules/_multiprocessing/semaphore.c

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,8 @@ shm_semlock_counters = {
390390
.shmlock_name = {0},
391391
.handle_shmlock = (SEM_HANDLE)0,
392392
.header = (HeaderObject *)NULL,
393-
.counters = (CounterObject *)NULL
393+
.counters = (CounterObject *)NULL,
394+
.max_open_sems = 0
394395
};
395396

396397
/*
@@ -435,12 +436,34 @@ _init_shm_names(pid_t pid)
435436
"%s%ld", SHMLOCK_NAME, (long)pid);
436437
}
437438

439+
static void
440+
_init_sem_max_from_kern_posix(int value)
441+
{
442+
shm_semlock_counters.max_open_sems = value/2;
443+
}
444+
438445
/*
439446
Public entry point called from multiprocessing_exec().
440-
Builds names from getpid() and exposes them as module attributes.
447+
Builds names from getpid(), get 'kern.posix.sem.max' value and exposes them as module attributes.
441448
*/
449+
442450
int
443-
_PyMp_init_shm_names(PyObject *module)
451+
_get_sem_max_from_kern_posix(void)
452+
{
453+
int value;
454+
size_t len = sizeof(value);
455+
456+
if (sysctlbyname("kern.posix.sem.max", &value, &len, NULL, 0) < 0) {
457+
value = sysconf(_SC_SEM_NSEMS_MAX);
458+
if (value < 0) {
459+
return 5000;
460+
}
461+
}
462+
return value;
463+
}
464+
465+
int
466+
_PyMp_init_module_constants(PyObject *module)
444467
{
445468
_init_shm_names(getpid());
446469
if (PyModule_AddStringConstant(module, "_MACOSX_SHAREDMEM_NAME",
@@ -449,6 +472,14 @@ _PyMp_init_shm_names(PyObject *module)
449472
if (PyModule_AddStringConstant(module, "_MACOSX_SHMLOCK_NAME",
450473
shm_semlock_counters.shmlock_name) < 0)
451474
return -1;
475+
476+
// A mutex is associated to each semaphore, so we could only open
477+
// value//2 MacOSX semaphores.
478+
int value = _get_sem_max_from_kern_posix();
479+
_init_sem_max_from_kern_posix(value);
480+
if (PyModule_AddIntConstant(module, "_MACOSX_MAX_OPEN_SEMS",
481+
value/2) < 0)
482+
return -1;
452483
return 0;
453484
}
454485

@@ -716,7 +747,7 @@ create_shm_semlock_counters(const char *from_sem_name) {
716747
header = shm_semlock_counters.header;
717748

718749
header->size_shm = size_shm;
719-
header->n_slots = CALC_NB_SLOTS(size_shm - sizeof(HeaderObject));
750+
header->n_slots = shm_semlock_counters.max_open_sems;
720751
header->n_semlocks = 0;
721752
header->sizeof_counter = sizeof(CounterObject);
722753
res = 0;
@@ -799,7 +830,7 @@ _search_counter_from_sem_name(const char *sem_name)
799830
HeaderObject *header = shm_semlock_counters.header;
800831
CounterObject *counter = shm_semlock_counters.counters;
801832

802-
for(int i = 0, j = 0; i < header->n_slots && j < header->n_semlocks; i++, counter++) {
833+
for(Py_ssize_t i = 0, j = 0; i < header->n_slots && j < header->n_semlocks; i++, counter++) {
803834
if (counter->sem_name[0] != 0) {
804835
if (!PyOS_stricmp(counter->sem_name, sem_name)) {
805836
return counter;
@@ -819,12 +850,11 @@ _search_counter_free_slot(void)
819850
HeaderObject *header = shm_semlock_counters.header;
820851
CounterObject *counter = shm_semlock_counters.counters;
821852

822-
for(int i = 0; i < header->n_slots; i++, counter++) {
853+
for(Py_ssize_t i = 0; i < header->n_slots; i++, counter++) {
823854
if(counter->sem_name[0] == 0) {
824855
return counter;
825856
}
826857
}
827-
828858
/*
829859
Not enough memory: see NSEMS_MAX in semaphore_macosx.h.
830860
*/
@@ -1175,6 +1205,7 @@ _multiprocessing_SemLock_impl(PyTypeObject *type, int kind, int value,
11751205
SEM_HANDLE handle = SEM_FAILED;
11761206
PyObject *result;
11771207
char *name_copy = NULL;
1208+
int err = 0;
11781209
#ifdef HAVE_BROKEN_SEM_GETVALUE
11791210
char mutex_name[SIZE_MUTEX_NAME];
11801211
SEM_HANDLE handle_mutex = SEM_FAILED;
@@ -1252,7 +1283,10 @@ _multiprocessing_SemLock_impl(PyTypeObject *type, int kind, int value,
12521283
failure:
12531284
#ifdef HAVE_BROKEN_SEM_GETVALUE
12541285
if (ISSEMAPHORE_FROM_ARGS(maxvalue, kind)) {
1286+
// Save errno before releasing the lock, as sem_post can change it
1287+
err = errno;
12551288
RELEASE_SHMLOCK;
1289+
errno = err;
12561290
}
12571291
failure_shmlock:
12581292
#endif /* HAVE_BROKEN_SEM_GETVALUE*/

Modules/_multiprocessing/semaphore_macosx.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#define SEMAPHORE_MACOSX_H
33

44
#include <unistd.h> // sysconf(SC_PAGESIZE)
5+
#include <sys/sysctl.h> // sysctlbyname
56

67
/*
78
Structures and constants in shared memory
@@ -33,13 +34,10 @@ typedef struct {
3334
Structure, constants and macros of static memory:
3435
*/
3536

36-
#define NSEMS_MAX sysconf(_SC_SEM_NSEMS_MAX) // returns 87381 on MacOSX 15.1 and m4 pro.
37-
#define CALC_SIZE_SHM (NSEMS_MAX * sizeof(CounterObject)) + sizeof(HeaderObject)
3837
#define SC_PAGESIZE sysconf(_SC_PAGESIZE)
38+
#define CALC_SIZE_SHM (shm_semlock_counters.max_open_sems * sizeof(CounterObject)) + sizeof(HeaderObject)
3939
#define ALIGN_SHM_PAGE(n) ((int)((n)/SC_PAGESIZE)+1)*SC_PAGESIZE
4040

41-
#define CALC_NB_SLOTS(n) (int)((((n)) - sizeof(HeaderObject)) / sizeof(CounterObject))
42-
4341
#define SHAREDMEM_NAME "/psm-gh125828-"
4442
#define SHMLOCK_NAME "/mp-gh125828-"
4543

@@ -54,6 +52,7 @@ struct _CountersWorkaround{
5452
MEMORY_HANDLE handle_shm; // Memory handle.
5553
char shmlock_name[SIZE_SHMLOCK_NAME];
5654
SEM_HANDLE handle_shmlock; // Global memory lock to handle shared memory.
55+
int max_open_sems; // Max number of opened semaphores.
5756
/*-- Pointers to shared memory --*/
5857
HeaderObject *header; // Pointer to header (shared memory).
5958
CounterObject*counters; // Pointer to the first item of fixed array (shared memory).

0 commit comments

Comments
 (0)