Changeset 1626

Show
Ignore:
Timestamp:
08/21/09 21:32:03 (14 years ago)
Author:
jwoithe
Message:

RME: refinement of shared memory management. Removal of race conditions (even though they were unlikely in practice).
RME: include shared memory management module in compilation in readiness for use.

Files:

Legend:

Unmodified
Added
Removed
Modified
Copied
Moved

  • trunk/libffado/src/rme/rme_shm.cpp

    r1624 r1626  
    4444#define RME_SHM_SIZE  sizeof(rme_shm_t) 
    4545 
     46#define RME_SHM_LOCKNAME "/ffado:rme_shm_lock" 
     47 
    4648#include <unistd.h> 
    4749#include <errno.h> 
    …… 
    5254#include "rme_shm.h" 
    5355 
     56static signed int rme_shm_lock(void) { 
     57signed lockfd; 
     58 
     59    do { 
     60        // The check for existance and shm creation are atomic so it's save 
     61        // to use this as the basis for a global lock. 
     62        lockfd = shm_open(RME_SHM_LOCKNAME, O_RDWR | O_CREAT | O_EXCL, 0644); 
     63        if (lockfd < 0) 
     64            usleep(10000); 
     65    } while (lockfd < 0); 
     66 
     67    return lockfd; 
     68} 
     69 
     70static void rme_shm_unlock(signed int lockfd) { 
     71    close(lockfd); 
     72    shm_unlink(RME_SHM_LOCKNAME); 
     73} 
     74 
    5475rme_shm_t *rme_shm_open(void) { 
    5576 
    56     signed int shmfd; 
     77    signed int shmfd, lockfd; 
    5778    rme_shm_t *data; 
    5879    signed int created = 0; 
    5980 
    60     // There is a small race condition here - if another RME FFADO process 
    61     // closes between the first and second shm_open() calls and in doing so 
    62     // removes the shm object, the current process will end up without a shm 
    63     // object since the second shm_open() call will fail.  At this point the 
    64     // practical likelihood of this is low, so we'll live with it. 
    65     shmfd = shm_open(RME_SHM_NAME, O_RDWR | O_CREAT | O_EXCL, 0644); 
     81    lockfd = rme_shm_lock(); 
     82 
     83    shmfd = shm_open(RME_SHM_NAME, O_RDWR, 0644); 
    6684    if (shmfd < 0) { 
    67         if (errno==EEXIST) { 
    68             shmfd = shm_open(RME_SHM_NAME, O_RDWR, 0644); 
    69             if (shmfd < 0)  
     85        if (errno == ENOENT) { 
     86            shmfd = shm_open(RME_SHM_NAME, O_RDWR | O_CREAT | O_EXCL, 0644); 
     87            if (shmfd < 0) 
    7088                return NULL; 
    71         } else { 
     89            else { 
     90                ftruncate(shmfd, RME_SHM_SIZE); 
     91                created = 1; 
     92            } 
     93        } else 
    7294            return NULL; 
    73         } 
    74     } else { 
    75         ftruncate(shmfd, RME_SHM_SIZE); 
    76         created = 1; 
    7795    } 
    7896 
    …… 
    87105    } 
    88106 
    89     // There's another unlikely race condition here.  If another process has 
    90     // done rme_shm_close() which results in a zero reference count between 
    91     // our second shm_open() call and here, we will be mapped to a shared 
    92     // object which is inaccessible to any subsequent processes due to the 
    93     // shm_unlink() call in rme_shm_close().  Again though, the practical 
    94     // changes of this occurring are low. 
    95107    pthread_mutex_lock(&data->lock); 
    96108    data->ref_count++; 
    …… 
    98110        data->valid++; 
    99111    pthread_mutex_unlock(&data->lock); 
     112 
     113    rme_shm_unlock(lockfd); 
    100114 
    101115    return data; 
    …… 
    105119 
    106120    signed int unlink = 0; 
     121    signed int lockfd; 
     122 
     123    lockfd = rme_shm_lock(); 
    107124 
    108125    pthread_mutex_lock(&shm_data->lock); 
    …… 
    112129    pthread_mutex_unlock(&shm_data->lock); 
    113130 
    114     // There's nothing to gain by explicitly calling pthread_mutex_destroy() 
    115     // on shm_data->lock, and doing so could cause issues for other processes 
    116     // which were waiting on the lock during the previous code block. 
     131    if (unlink) { 
     132        // This is safe: if the reference count is zero there can't be any 
     133        // other process using the lock at this point. 
     134        pthread_mutex_destroy(&shm_data->lock); 
     135    } 
    117136 
    118137    munmap(shm_data, RME_SHM_SIZE); 
    …… 
    120139    if (unlink) 
    121140        shm_unlink(RME_SHM_NAME); 
     141 
     142    rme_shm_unlock(lockfd); 
    122143} 

  • trunk/libffado/src/SConscript

    r1599 r1626  
    170170 
    171171rme_source = env.Split( '\ 
     172        rme/rme_shm.cpp \ 
    172173        rme/rme_avdevice.cpp \ 
    173174        rme/rme_avdevice_settings.cpp \