WriteToClient() can be called from XIChangeDeviceProperty() so from the InputThread which is a problem as it allocates and free the input and output buffers.
Add a new mutex to protect accesses to the input and output buffers from being accessed from different threads. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99164 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99887 Signed-off-by: Olivier Fourdan <[email protected]> --- RFC: This is probably sub-optimal and broken in many places, but it seems to avoid the memory corruption (so far)... At least it's a start, I guess. os/io.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 66 insertions(+), 7 deletions(-) diff --git a/os/io.c b/os/io.c index be85226..b0e3825 100644 --- a/os/io.c +++ b/os/io.c @@ -107,6 +107,36 @@ static ConnectionInputPtr FreeInputs = (ConnectionInputPtr) NULL; static ConnectionOutputPtr FreeOutputs = (ConnectionOutputPtr) NULL; static OsCommPtr AvailableInput = (OsCommPtr) NULL; +/* iobuf_mutex code copied from inputthread.c */ +#ifdef PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP +static pthread_mutex_t iobuf_mutex = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP; +#else +static pthread_mutex_t iobuf_mutex; +static Bool iobuf_mutex_initialized; +#endif + +static void +iobuf_lock(void) +{ +#ifndef PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP + if (!iobuf_mutex_initialized) { + pthread_mutexattr_t mutex_attr; + + iobuf_mutex_initialized = TRUE; + pthread_mutexattr_init(&mutex_attr); + pthread_mutexattr_settype(&mutex_attr, PTHREAD_MUTEX_RECURSIVE); + pthread_mutex_init(&iobuf_mutex, &mutex_attr); + } +#endif + pthread_mutex_lock(&iobuf_mutex); +} + +static void +iobuf_unlock(void) +{ + pthread_mutex_unlock(&iobuf_mutex); +} + #define get_req_len(req,cli) ((cli)->swapped ? \ lswaps((req)->length) : (req)->length) @@ -203,6 +233,7 @@ YieldControlDeath(void) static void NextAvailableInput(OsCommPtr oc) { + iobuf_lock(); if (AvailableInput) { if (AvailableInput != oc) { ConnectionInputPtr aci = AvailableInput->input; @@ -219,6 +250,7 @@ NextAvailableInput(OsCommPtr oc) } AvailableInput = NULL; } + iobuf_unlock(); } int @@ -237,12 +269,14 @@ ReadRequestFromClient(ClientPtr client) /* make sure we have an input buffer */ + iobuf_lock(); if (!oci) { if ((oci = FreeInputs)) { FreeInputs = oci->next; } else if (!(oci = AllocateInputBuffer())) { YieldControlDeath(); + iobuf_unlock(); return -1; } oc->input = oci; @@ -313,6 +347,7 @@ ReadRequestFromClient(ClientPtr client) */ oci->ignoreBytes = needed - gotnow; oci->lenLastReq = gotnow; + iobuf_unlock(); return needed; } if ((gotnow == 0) || ((oci->bufptr - oci->buffer + needed) > oci->size)) { @@ -346,6 +381,7 @@ ReadRequestFromClient(ClientPtr client) * used to happen */ YieldControlDeath(); + iobuf_unlock(); return -1; } result = _XSERVTransRead(oc->trans_conn, oci->buffer + oci->bufcnt, @@ -358,10 +394,12 @@ ReadRequestFromClient(ClientPtr client) #endif { YieldControlNoInput(fd); + iobuf_unlock(); return 0; } } YieldControlDeath(); + iobuf_unlock(); return -1; } oci->bufcnt += result; @@ -395,6 +433,7 @@ ReadRequestFromClient(ClientPtr client) if (gotnow < needed) { /* Still don't have enough; punt. */ YieldControlNoInput(fd); + iobuf_unlock(); return 0; } } @@ -447,6 +486,8 @@ ReadRequestFromClient(ClientPtr client) client->req_len -= bytes_to_int32(sizeof(xBigReq) - sizeof(xReq)); } client->requestBuffer = (void *) oci->bufptr; + iobuf_unlock(); + #ifdef DEBUG_COMMUNICATION { xReq *req = client->requestBuffer; @@ -498,12 +539,14 @@ InsertFakeRequest(ClientPtr client, char *data, int count) int gotnow, moveup; NextAvailableInput(oc); - + iobuf_lock(); if (!oci) { if ((oci = FreeInputs)) FreeInputs = oci->next; - else if (!(oci = AllocateInputBuffer())) + else if (!(oci = AllocateInputBuffer())) { + iobuf_unlock(); return FALSE; + } oc->input = oci; } oci->bufptr += oci->lenLastReq; @@ -513,8 +556,10 @@ InsertFakeRequest(ClientPtr client, char *data, int count) char *ibuf; ibuf = (char *) realloc(oci->buffer, gotnow + count); - if (!ibuf) + if (!ibuf) { + iobuf_unlock(); return FALSE; + } oci->size = gotnow + count; oci->buffer = ibuf; oci->bufptr = ibuf + oci->bufcnt - gotnow; @@ -534,6 +579,7 @@ InsertFakeRequest(ClientPtr client, char *data, int count) mark_client_ready(client); else YieldControlNoInput(fd); + iobuf_unlock(); return TRUE; } @@ -698,7 +744,7 @@ WriteToClient(ClientPtr who, int count, const void *__buf) multicount = TRUE; } #endif - + iobuf_lock(); if (!oco) { if ((oco = FreeOutputs)) { FreeOutputs = oco->next; @@ -710,6 +756,7 @@ WriteToClient(ClientPtr who, int count, const void *__buf) oc->trans_conn = NULL; } MarkClientException(who); + iobuf_unlock(); return -1; } oc->output = oco; @@ -763,7 +810,7 @@ WriteToClient(ClientPtr who, int count, const void *__buf) CriticalOutputPending = FALSE; NewOutputPending = FALSE; } - + iobuf_unlock(); return FlushClient(who, oc, buf, count); } @@ -775,6 +822,7 @@ WriteToClient(ClientPtr who, int count, const void *__buf) memset(oco->buf + oco->count, '\0', padBytes); oco->count += padBytes; } + iobuf_unlock(); return count; } @@ -805,10 +853,12 @@ FlushClient(ClientPtr who, OsCommPtr oc, const void *__extraBuf, int extraCount) return 0; written = 0; padsize = padding_for_int32(extraCount); + iobuf_lock(); notWritten = oco->count + extraCount + padsize; - if (!notWritten) + if (!notWritten) { + iobuf_unlock(); return 0; - + } if (FlushCallback) CallCallbacks(&FlushCallback, who); @@ -894,6 +944,7 @@ FlushClient(ClientPtr who, OsCommPtr oc, const void *__extraBuf, int extraCount) oc->trans_conn = NULL; MarkClientException(who); oco->count = 0; + iobuf_unlock(); return -1; } oco->size = notWritten + BUFSIZE; @@ -908,6 +959,7 @@ FlushClient(ClientPtr who, OsCommPtr oc, const void *__extraBuf, int extraCount) oco->count = notWritten; /* this will include the pad */ ospoll_listen(server_poll, oc->fd, X_NOTIFY_WRITE); + iobuf_unlock(); /* return only the amount explicitly requested */ return extraCount; @@ -925,6 +977,7 @@ FlushClient(ClientPtr who, OsCommPtr oc, const void *__extraBuf, int extraCount) } MarkClientException(who); oco->count = 0; + iobuf_unlock(); return -1; } } @@ -942,6 +995,8 @@ FlushClient(ClientPtr who, OsCommPtr oc, const void *__extraBuf, int extraCount) FreeOutputs = oco; } oc->output = (ConnectionOutputPtr) NULL; + iobuf_unlock(); + return extraCount; /* return only the amount explicitly requested */ } @@ -990,6 +1045,7 @@ FreeOsBuffers(OsCommPtr oc) ConnectionInputPtr oci; ConnectionOutputPtr oco; + iobuf_lock(); if (AvailableInput == oc) AvailableInput = (OsCommPtr) NULL; if ((oci = oc->input)) { @@ -1017,6 +1073,7 @@ FreeOsBuffers(OsCommPtr oc) oco->count = 0; } } + iobuf_unlock(); } void @@ -1025,6 +1082,7 @@ ResetOsBuffers(void) ConnectionInputPtr oci; ConnectionOutputPtr oco; + iobuf_lock(); while ((oci = FreeInputs)) { FreeInputs = oci->next; free(oci->buffer); @@ -1035,4 +1093,5 @@ ResetOsBuffers(void) free(oco->buf); free(oco); } + iobuf_unlock(); } -- 2.9.3 _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
