[Openais] [patch] saEvtDispatch seg fault
Russell Bryant
russell at digium.com
Tue Aug 14 07:22:53 PDT 2007
Steven Dake wrote:
> The initialize function should initialize any data areas needed for
> operation for APIs that cannot return SA_AIS_ERR_NO_MEMORY.
> =
> If you want to modify your patch to do that it would be much appreciated
> - or I should get to it at some point in the future after my current
> plate of bugs clears up.
I have attached a patch that addresses your recommendations. It does the f=
ollowing:
* Moves the 1 MB dispatch_data that was allocated on the stack in evtDispat=
ch, =
to be a member of the event_instance struct.
* Tweaks the locking in evtDispatch such that the dispatch_mutex also ensur=
es =
that only one thread is using the dispatch_data buffer.
- Unfortunately, this means holding the mutex longer than before. The on=
ly =
way I see to improve this would be to use a thread local buffer here, inste=
ad. =
However, that would put us back to doing the allocation in evtDispatch inst=
ead =
of evtInitialize.
The patch was made against the whitetank branch, but will also apply to tru=
nk.
-- =
Russell Bryant
Software Engineer
Digium, Inc.
-------------- next part --------------
Index: lib/evt.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- lib/evt.c (revision 1420)
+++ lib/evt.c (working copy)
@@ -110,10 +110,12 @@
* ei_version: version sent to the evtInitialize call.
* ei_node_id: our node id.
* ei_node_name: our node name.
- * ei_finalize: instance in finalize flag
- * ei_dispatch_mutex: mutex for dispatch fd
+ * ei_dispatch_mutex: mutex for dispatch fd. This lock also ensures that
+ * only one thread is using ei_dispatch_data.
* ei_response_mutex: mutex for response fd
* ei_channel_list: list of associated channels (struct handle_list)
+ * ei_dispatch_data: event buffer for evtDispatch
+ * ei_finalize: instance in finalize flag
* ei_data_available: Indicates that there is a pending event message thou=
gh
* there may not be a poll event. This can happen
* when we get a SA_AIS_ERR_TRY_AGAIN when asking for an
@@ -127,11 +129,12 @@
SaVersionT ei_version;
SaClmNodeIdT ei_node_id;
SaNameT ei_node_name;
- int ei_finalize;
pthread_mutex_t ei_dispatch_mutex;
pthread_mutex_t ei_response_mutex;
struct list_head ei_channel_list;
- int ei_data_available;
+ struct res_overlay ei_dispatch_data;
+ unsigned int ei_finalize:1;
+ unsigned int ei_data_available:1;
};
=
=
@@ -605,7 +608,6 @@
int ignore_dispatch =3D 0;
int cont =3D 1; /* always continue do loop except when set to 0 */
int poll_fd;
- struct res_overlay dispatch_data;
struct lib_event_data *evt =3D 0;
struct res_evt_event_data res;
=
@@ -680,15 +682,15 @@
}
=
if (ufds.revents & POLLIN) {
- error =3D saRecvRetry (evti->ei_dispatch_fd, &dispatch_data.header,
+ error =3D saRecvRetry (evti->ei_dispatch_fd, &evti->ei_dispatch_data.h=
eader,
sizeof (mar_res_header_t));
=
if (error !=3D SA_AIS_OK) {
goto dispatch_unlock;
}
- if (dispatch_data.header.size > sizeof (mar_res_header_t)) {
- error =3D saRecvRetry (evti->ei_dispatch_fd, &dispatch_data.data,
- dispatch_data.header.size - sizeof (mar_res_header_t));
+ if (evti->ei_dispatch_data.header.size > sizeof (mar_res_header_t)) {
+ error =3D saRecvRetry (evti->ei_dispatch_fd, &evti->ei_dispatch_data.=
data,
+ evti->ei_dispatch_data.header.size - sizeof (mar_res_header_t));
if (error !=3D SA_AIS_OK) {
goto dispatch_unlock;
}
@@ -703,7 +705,7 @@
* Fake up a header message and the switch statement will
* take care of the rest.
*/
- dispatch_data.header.id =3D MESSAGE_RES_EVT_AVAILABLE;
+ evti->ei_dispatch_data.header.id =3D MESSAGE_RES_EVT_AVAILABLE;
}
=
/*
@@ -713,13 +715,11 @@
* EvtFinalize has been called in another thread.
*/
memcpy(&callbacks, &evti->ei_callback, sizeof(evti->ei_callback));
- pthread_mutex_unlock(&evti->ei_dispatch_mutex);
=
-
/*
* Dispatch incoming response
*/
- switch (dispatch_data.header.id) {
+ switch (evti->ei_dispatch_data.header.id) {
=
case MESSAGE_RES_EVT_AVAILABLE:
evti->ei_data_available =3D 0;
@@ -788,7 +788,7 @@
case MESSAGE_RES_EVT_CHAN_OPEN_CALLBACK:
{
struct res_evt_open_chan_async *resa =3D =
- (struct res_evt_open_chan_async *)&dispatch_data;
+ (struct res_evt_open_chan_async *)&evti->ei_dispatch_data;
struct event_channel_instance *eci;
=
/*
@@ -821,11 +821,13 @@
break;
=
default:
- DPRINT (("Dispatch: Bad message type 0x%x\n", dispatch_data.header.id));
+ DPRINT (("Dispatch: Bad message type 0x%x\n", evti->ei_dispatch_data.he=
ader.id));
error =3D SA_AIS_ERR_LIBRARY; =
- goto dispatch_put;
+ goto dispatch_unlock;
}
=
+ pthread_mutex_unlock(&evti->ei_dispatch_mutex);
+
/*
* If empty is zero it means the we got the =
* message from the queue and we are responsible
@@ -860,7 +862,7 @@
goto dispatch_put;
=
dispatch_unlock:
- pthread_mutex_unlock(&evti->ei_dispatch_mutex);
+ pthread_mutex_unlock(&evti->ei_dispatch_mutex);
dispatch_put:
saHandleInstancePut(&evt_instance_handle_db, evtHandle);
return error;
More information about the Openais
mailing list