pending_semops variable can be changed in two threads. One is actual IPC
connection and second is coropoll. It's really scholar example of race
(one thread doing i++, second doing i--). If socket is full, it can
happen that IPC will increase value and coropoll will decrease,
resulting in unpredictable value. This means, that client IPC can be
informed about more messages then really available, resulting
in reading of garbage messages in library dispatch function.
Solution is to properly lock variable.
Signed-off-by: Jan Friesse <jfriesse@xxxxxxxxxx>
---
exec/coroipcs.c | 27 +++++++++++++++------------
1 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/exec/coroipcs.c b/exec/coroipcs.c
index 36d18a4..a5c3248 100644
--- a/exec/coroipcs.c
+++ b/exec/coroipcs.c
@@ -140,6 +140,7 @@ struct conn_info {
key_t semkey;
#endif
unsigned int pending_semops;
+ pthread_mutex_t pending_semops_mutex;
pthread_mutex_t mutex;
struct control_buffer *control_buffer;
char *request_buffer;
@@ -1265,7 +1266,9 @@ static void msg_send (void *conn, const struct iovec *iov, unsigned int iov_len,
buf = list_empty (&conn_info->outq_head);
res = send (conn_info->fd, &buf, 1, MSG_NOSIGNAL);
if (res != 1) {
+ pthread_mutex_lock(&conn_info->pending_semops_mutex);
conn_info->pending_semops += 1;
+ pthread_mutex_unlock(&conn_info->pending_semops_mutex);
if (conn_info->poll_state == POLL_STATE_IN) {
conn_info->poll_state = POLL_STATE_INOUT;
api->poll_dispatch_modify (conn_info->fd,
@@ -1648,6 +1651,7 @@ int coroipcs_handler_dispatch (
}
pthread_mutex_init (&conn_info->mutex, NULL);
+ pthread_mutex_init (&conn_info->pending_semops_mutex, NULL);
req_setup = (mar_req_setup_t *)conn_info->setup_msg;
/*
* Is the service registered ?
@@ -1784,22 +1788,21 @@ int coroipcs_handler_dispatch (
}
if (revent & POLLOUT) {
- int psop = conn_info->pending_semops;
- int i;
-
- assert (psop != 0);
- for (i = 0; i < psop; i++) {
- res = send (conn_info->fd, &buf, 1, MSG_NOSIGNAL);
- if (res != 1) {
- return (0);
- } else {
- conn_info->pending_semops -= 1;
- }
+ pthread_mutex_lock(&conn_info->pending_semops_mutex);
+
+ assert(conn_info->pending_semops != 0);
+
+ while (conn_info->pending_semops > 0 &&
+ ((res = send (conn_info->fd, &buf, 1, MSG_NOSIGNAL)) == 1)) {
+
+ conn_info->pending_semops -= 1;
}
- if (conn_info->poll_state == POLL_STATE_INOUT) {
+
+ if (conn_info->pending_semops == 0 && conn_info->poll_state == POLL_STATE_INOUT) {
conn_info->poll_state = POLL_STATE_IN;
api->poll_dispatch_modify (conn_info->fd, POLLIN|POLLNVAL);
}
+ pthread_mutex_unlock(&conn_info->pending_semops_mutex);
}
return (0);
--
1.7.1
_______________________________________________
discuss mailing list
discuss@xxxxxxxxxxxx
http://lists.corosync.org/mailman/listinfo/discuss
