Opened 5 years ago

Closed 4 years ago

#11208 closed bug (fixed)

POSIX semaphores have incorrect sharing semantics

Reported by: hamish Owned by: nobody
Priority: normal Milestone: Unscheduled
Component: System/POSIX Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

POSIX semaphores should only be shared between processes if a non-zero pshared value is given to sem_init, and the semaphore resides in a shared region of memory (like a PTHREAD_PROCESS_SHARED mutex). The POSIX spec is not explicit about it, but this is how Linux and FreeBSD behave.

Haiku ignores the latter requirement, so pshared semaphores in private memory (e.g. the stack or heap) are shared after a fork.

Test case:

#include <semaphore.h>
#include <unistd.h>

#include <assert.h>
#include <stdio.h>


int main() {
    sem_t sem;
    int res = sem_init(&sem, 1, 0);
    assert(res == 0);

    int pid = fork();
    assert(pid != -1);

    if (pid == 0) {
        res = sem_post(&sem);
        assert(res == 0);
    } else {
        res = sem_wait(&sem);
        assert(res == 0);
    }

    printf("End\n");
    return 0;
}

The parent process should wait indefinitely on the semaphore, but it does not.

Attachments (1)

0001-Reimplement-unnamed-POSIX-semaphores-using-user_mute.patch (21.2 KB) - added by hamish 4 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 4 years ago by luroh

Milestone: R1Unscheduled

Move POSIX compatibility related tickets out of R1 milestone (FutureHaiku/Features).

comment:2 Changed 4 years ago by hamish

Has a Patch: set

comment:3 Changed 4 years ago by hamish

Attached is a patch which reimplements unnamed POSIX semaphores using the user_mutex hashtable wait mechanism. Two new system calls are added: _user_mutex_sem_acquire and _user_mutex_sem_release.

Unnamed semaphores now consist of an int32_t in userspace. Acquires and releases are done using atomic ops in userspace when the semaphore has no waiters (when the semaphores value is >= 0). When the semaphore is contended the value is set to -1 and the system calls are used to acquire or release.

I've run the POSIX test suite stress tests for semaphores and all seems well, however I thought I'd post it here for a second opinion first.

comment:4 Changed 4 years ago by axeld

I'm not familiar with the user mutexes at all, I just wonder why _user_mutex_sem_{acquire|release}() is a syscall, just looking at what it does.

comment:5 Changed 4 years ago by hamish

They need to be syscalls because they enqueue (or dequeue) the thread in the global sUserMutexTable hashtable (see user_mutex_wait_locked in the patch) for later waking up, which enables implementation of process shared mutexes and semaphores.

The reason for repeating the atomic ops in kernel space is because it's done with the sUserMutexTableLock lock held, to prevent a race with someone else releasing the semaphore. With the lock held, a thread acquiring the semaphore will set it to -1, to mark it as contended. Any thread releasing the semaphore will then notice this value and use the system call to release it, and take the same lock when waking up the original thread.

comment:6 Changed 4 years ago by hamish

Resolution: fixed
Status: newclosed

Applied in hrev49190.

Note: See TracTickets for help on using tickets.