[Libreoffice] Mutex(Guard) ignores failure to create/acquire Mutex

[Norbert Thiebaud]

On Thu, Aug 18, 2011 at 12:12 AM, Lionel Elie Mamane <lionel at mamane.lu> wrote:
> Hi,
>
> I noticed that Mutex(Guard) C++ helper classes silently ignores
> failures to create / acquire / release / destroy a mutex, which seems
> rather worrying: if one puts a mutex acquire, it means the rest of the
> code should not execute if the mutex was not acquired, lest subtle
> bugs crop up (corruption of thread-shared variables and all that).
>
> So my natural tendency would be to make create (in Mutex::Mutex) and
> acquire (in Guard::Guard, ResettableGuard::reset, etc) errors "hard"
> errors by throwing a RuntimeException; that is, obviously, unless the
> file is compiled without exception support, in which case... we
> fallback to the current behaviour? Or rather an OSL_FAIL?
>
> Release errors in a MutexGuard destructor cannot be an exception, so
> an OSL_FAIL.
>
> The Mutex/MutexGuard classes are implemented 100% in header file, so
> they can adapt to the compilation options of the file they are used
> in, and it seems the LO build system nicely defines EXCEPTIONS_ON or
> EXCEPTIONS_OFF to give that information.
>
>
> Opinions? Any reason this is a bad idea?

let's look at the man page of unixes, pthread_mutex_lock can fail on:

      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.

we don't do prio-protect... so n.a.


      The pthread_mutex_lock(), pthread_mutex_trylock(), and
pthread_mutex_unlock() functions may fail if:

       EINVAL The value specified by mutex does not refer to an
initialized mutex object.
that is very unlikely with a c++ class

       EAGAIN The mutex could not be acquired because the maximum
number of recursive locks for mutex has been exceeded.
ok, that one is remotely possible, we do use recursive mutex (yeah,
ugly I know :-( ) and I don't thing we pay any attention to the
maximum nesting level...
on the other hand, in glibc that limit is an overflow of __count which
is defined as an int, so 2^32 recursive lock before overflow... I
think you are going to blow for other reasons way before that.



       The pthread_mutex_lock() function may fail if:

       EDEADLK
              The current thread already owns the mutex.
That can't happen, because we do use recursive mutex


       The pthread_mutex_unlock() function may fail if:

       EPERM  The current thread does not own the mutex.
that is very-very unlikey we a c++ guard class

for create.. more exactly init
     The pthread_mutex_init() function shall fail if:

       EAGAIN The system lacked the necessary resources (other than
memory) to initialize another mutex.

       ENOMEM Insufficient memory exists to initialize the mutex.

These 2: you have bigger problems.... you're going to die soon anyway...

       EPERM  The caller does not have the privilege to perform the operation.

I'm not sure what can trigger that one...


       The pthread_mutex_init() function may fail if:

       EBUSY  The implementation has detected an attempt to
reinitialize the object referenced by mutex, a previously initialized,
but not yet destroyed, mutex.

       EINVAL The value specified by attr is invalid.

These two are highly unlikely with a Guad c++ class.


       The pthread_mutex_destroy() function may fail if:

       EBUSY  The implementation has detected an attempt to destroy
the object referenced by mutex while it is locked or referenced (for
example, while being used in a pthread_cond_timedwait() or
pthread_cond_wait()) by another thread.

       EINVAL The value specified by mutex is invalid.

again very unlikely with a c++ guard class.

on windows we use CriticalSection to implement osl_mutex.
EnterCriticalSection does not return a value... it is recursive but
the doc does not mention any limit (and since Windows is not open
source, I cannot go and look in the source code to find out what the
limit is)
LeaveCritical section also return void (altough the doc says: "If a
thread calls LeaveCriticalSection when it does not have ownership of
the specified critical section object, an error occurs that may cause
another thread using EnterCriticalSection to wait indefinitely." great
an error _can_ occur, but you cannot test for it, because the function
return void.... ahh Windows :-)
InitializeCriticalSection used to be able to have low-ressource error
mode... but not anymore since Vista...

so bottom line, there are no failure mode that we can do anything
about in windows...


Guards are used and badly abused in the code, especially with the
infamous SolarMutex...
anything that would make that code slower or fatter than it already is
(with virtual functions and all), especially for corner-case
'you-are-about-to-die-anyway' scenario, is bad.

so my opinion is, just let it be :-)

Norbert