philippe44 / LMS-uPnP

Integrate UPnP players with LogitechMediaServer
108 stars 17 forks source link

A possible locking issue #56

Closed ryancaicse closed 2 years ago

ryancaicse commented 2 years ago

Hi, developers, the lock p->Mutex seems not be acquired or released in the method FlushMRDevices https://github.com/philippe44/LMS-to-uPnP/blob/4e66556e926b889a58440a3dea4624d313b80179/application/squeeze2upnp/mr_util.c#L155-L160

philippe44 commented 2 years ago

It is in the DelMRDevice

ryancaicse commented 2 years ago

it wouldn’t cause any problems?

philippe44 commented 2 years ago

No because there is always a proper lock/unlock pair. The unlock is either done by the delMR... or in the loop if the player does not run

ryancaicse commented 2 years ago

Thanks so much for your explanations.

philippe44 commented 2 years ago

I agree it's not the most elegant, but there was a reason (I hope...) that I can't remember why I do an else and don't simply unlock in all cases after the call to DelMRDevice,

ryancaicse commented 2 years ago

@philippe44 Also, I have a suggestion. Would it be better to use a lock wrapper that handles the possible errors from pthread_mutex_lock.

This example does not check the value returned by pthread_mutex_lock() for errors. If pthread_mutex_lock() cannot acquire the mutex for any reason, the function may introduce a race condition into the program.

void f(pthread_mutex_t *mutex) {
pthread_mutex_lock(mutex);

/* access shared resource */

pthread_mutex_unlock(mutex);
}