[PATCH 1/1] dix: Hold the input_lock() when accessing the miEventQueue through checkForInput

[Jeremy Huddleston Sequoia]

> On Sep 19, 2016, at 09:52, Keith Packard <keithp at keithp.com> wrote:
> 
> Jeremy Huddleston Sequoia <jeremyhu at apple.com> writes:
> 
>> Yeah, I made the change mainly to shutup the analyzer while I was
>> looking for other races.  I decided to propose it in case we want to
>> be strict here.
> 
> Thanks. Keeping our request processing overhead low seems important
> enough to me that we should skip this patch, although perhaps adding a
> comment before the test noting that we are intentionally not holding the
> input lock.
> 
> Is there some way we can annotate the code to silence the analyzer in
> these cases?

The annotation is at a per-function level.  We can basically say "if you notice data races in this funciton, don't bother reporting them".  I'm doing this for a similar case where we want to limit overhead on the fastpath in darwinEvents.c.  In the code below, TSan would normally complain about the read of mieqInitialized without holding the mieqInitializedMutex, but we know it's actually safe in this case.

We could add something like _X_NOTSAN to Xfuncproto.h to cover this and then break out the actual comparisons into an inline function with that attribute.  Breaking that out into separate inline functions allows us to surgically apply no_sanitize_thread. 

Thoughts?


static BOOL mieqInitialized;
static pthread_mutex_t mieqInitializedMutex = PTHREAD_MUTEX_INITIALIZER;
static pthread_cond_t mieqInitializedCond = PTHREAD_COND_INITIALIZER;

#ifdef __has_feature
# if __has_feature(thread_sanitizer)
#  define __tsan_ignore __attribute__((no_sanitize_thread))
# else
#  define __tsan_ignore /**/
# endif
#else
# define __tsan_ignore /**/
#endif

__tsan_ignore
extern inline void
wait_for_mieq_init(void)
{
    if (!mieqInitialized) {
        pthread_mutex_lock(&mieqInitializedMutex);
        while (!mieqInitialized) {
            pthread_cond_wait(&mieqInitializedCond, &mieqInitializedMutex);
        }
        pthread_mutex_unlock(&mieqInitializedMutex);
    }
}

__tsan_ignore
static inline void
signal_mieq_init(void)
{
    pthread_mutex_lock(&mieqInitializedMutex);
    mieqInitialized = TRUE;
    pthread_cond_broadcast(&mieqInitializedCond);
    pthread_mutex_unlock(&mieqInitializedMutex);
}

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4465 bytes
Desc: not available
URL: <https://lists.x.org/archives/xorg-devel/attachments/20160919/64104f04/attachment.bin>