Bug 596351 - Review of mutex locking in libs
Summary: Review of mutex locking in libs
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Red Hat Enterprise Linux 5
Classification: Red Hat
Component: openais
Version: 5.5
Hardware: All
OS: Linux
low
medium
Target Milestone: rc
: ---
Assignee: Angus Salkeld
QA Contact: Cluster QE
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-05-26 16:10 UTC by Jan Friesse
Modified: 2011-08-05 14:24 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-08-05 14:24:21 UTC
Target Upstream Version:


Attachments (Terms of Use)
untested proposal patch (35.14 KB, patch)
2010-06-20 01:25 UTC, Angus Salkeld
no flags Details | Diff

Description Jan Friesse 2010-05-26 16:10:40 UTC
Description of problem:
Locking in upstream whitetank in some libraries (like evt, msg, ...) really doesn't look correct.

As example, take a look to lib/evt.c:632. Dispatch mutex is UNlocked, but it is never locked.

This *can* be problem with multi-threaded applications.

Version-Release number of selected component (if applicable):
Upstream whitetank

Comment 2 Angus Salkeld 2010-06-20 01:25:01 UTC
Created attachment 425407 [details]
untested proposal patch

Ya, these need to cleaned up.

Tell me if i have any think wrong here.

The dispatch_mutex is there to protect the dispatch buffer and file descriptor.

So this needs to be locked for the openais_dispatch_recv() call (as
it writes to the buffer) and until after the callback gets called
(so we don't have to copy bits out of the instance struct).

So the other issue here is the finalize.
I think we can solve this by taking the dispatch lock in the finalize function
(as well as the response lock) to make sure the instance is not freed during
a dispatch call.

I have attached a patch which makes all the libs consistent with
the basic flow as below:
dispatch()
{

        do {
            	lock();
                if (inst->finalize) {
                        goto error_unlock;
               	}
                recv();
                callback();
                if (cont) {
                        unlock();
               	}
        while (cont);
error_unlock:
        unlock();
        put();
}

finalize()
{
        lock(dispatch & response);
        inst->finalize = 1;
       	...
	unlock(dispatch & response);
}

A potential problem is the ordering of these locks could result in a dead lock:
(evt locks the response lock within the dispatch loop).

dispatch:
lock(1);
lock(2);

finalize:
lock(2);
lock(1);

Comment 3 RHEL Program Management 2011-01-11 20:51:58 UTC
This request was evaluated by Red Hat Product Management for
inclusion in the current release of Red Hat Enterprise Linux.
Because the affected component is not scheduled to be updated in the
current release, Red Hat is unfortunately unable to address this
request at this time. Red Hat invites you to ask your support
representative to propose this request, if appropriate and relevant,
in the next release of Red Hat Enterprise Linux.

Comment 4 RHEL Program Management 2011-01-11 23:15:43 UTC
This request was erroneously denied for the current release of
Red Hat Enterprise Linux.  The error has been fixed and this
request has been re-proposed for the current release.


Note You need to log in before you can comment on or make changes to this bug.