archiecobbs / s3backer

FUSE/NBD single file backing store via Amazon S3
Other
535 stars 77 forks source link

Suggestions about error handlings for locking #156

Closed ryancaicse closed 2 years ago

ryancaicse commented 2 years ago

Hi, developers, I have a suggestion about error handlings for locking. Would it be better to handle the possible errors that return from pthread_mutex_lock.

For example, 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 (CWE-413).

The manners of error handlings could be flagging any warnings or returning before accessing the critical region.

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

/* access shared resource */

pthread_mutex_unlock(mutex);
}
archiecobbs commented 2 years ago

From pthread_mutex_lock the possible causes of error return are:

       EAGAIN The mutex could not be acquired because the maximum number
              of recursive locks for mutex has been exceeded.

       EINVAL The mutex was created with the protocol attribute having
              the value PTHREAD_PRIO_PROTECT and the calling thread's
              priority is higher than the mutex's current priority
              ceiling.

       ENOTRECOVERABLE
              The state protected by the mutex is not recoverable.

       EOWNERDEAD
              The mutex is a robust mutex and the process containing the
              previous owning thread terminated while holding the mutex
              lock. The mutex lock shall be acquired by the calling
              thread and it is up to the new owner to make the state
              consistent.

       EDEADLK
              The mutex type is PTHREAD_MUTEX_ERRORCHECK and the current
              thread already owns the mutex.

       EOWNERDEAD
              The mutex is a robust mutex and the previous owning thread
              terminated while holding the mutex lock. The mutex lock
              shall be acquired by the calling thread and it is up to
              the new owner to make the state consistent.

       EDEADLK
              A deadlock condition was detected.

So the only way an error could be returned from pthread_mutex_lock() would be due to a bug in the code.

So that means "fixing" this is lower priority.

The usual way to react in such a scenario is to bail out with an error, e.g., using assert(r == 0).

So we could create a wrapper macro that does this. See for example issue156.

ryancaicse commented 2 years ago

Thank you!

archiecobbs commented 2 years ago

Merged into master branch in 8c0b12c.