hellerf / EmbeddableWebServer

Cross-platform, single .h file HTTP server (Windows, Linux, Mac OS X)
https://forrestheller.com/embeddable-c-web-server
102 stars 13 forks source link

Error handlings for pthread_mutex_lock #11

Open ycaibb opened 2 years ago

ycaibb 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.

Possible situations that return errors.

    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.

https://github.com/hellerf/EmbeddableWebServer/blob/9d71cee513e2eae98d1676b554385c7e894096db/EmbeddableWebServer.h#L1920-L1922

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);
}
hellerf commented 2 years ago

Hello yccaib, Thanks for writing in. If you are concerned about pthread_mutex return values not being checked, this is probably not the web server for you. I'm guessing there are more egregious correctness issues.

However, I would take a pull request that handles the mutex locking failures. Besides EAGAIN, I'm guessing you would want to:

  1. Define some debug option like OptionCrashOnMutexLockFailure (or something) and have it default to false/off
  2. If a failure is detected, consult OptionCrashOnMutexLockFailure. If it is true, crash the program. Otherwise, make a ews_printf() with information about the failure.

Although really I'm guessing we would just want an ews_printf() in that case, and no users would ever set OptionCrashOnMutexLockFailure to true.

I'm not really sure there's an option to gracefully recover from the pthread_mutex lock failure. And it should be extremely rare.

I will say that the only time I have ever seen pthread_mutex functions return non-zero are when I forget to initialize them.

Although I would rather someone add support for handling the HTTP Range: header. That would be preferred and cool.