[PATCH] client: Add acquire-fd API to avoid requiring a polling main thread

[Kristian Høgsberg]

On Mon, Mar 25, 2013 at 09:42:11PM +0100, Uli Schlachter wrote:
> On 25.03.2013 21:33, Thiago Macieira wrote:
> > On segunda-feira, 25 de março de 2013 19.49.32, Uli Schlachter wrote:
> >> So wl_display_acquire_fd() would do:
> >>
> >>     if (old_state == VOLUNTEER_READER) {
> >>         write(display->reader_pipe[1], &c, 1);
> >>         pthread_cond_wait(&display->pipe_cond, &display->mutex);
> >>         read(display->reader_pipe[0], &c, 1);
> >>     }
> > 
> > There might be a priority inversion / resource starvation problem here.
> > 
> > When the other thread returns to its caller, that caller may call the dispatch 
> > function again and cause it to poll(2) again, before the pipe is emptied. 
> > Since the pipe is still readable, poll(2) will return immediately and it 
> > returns to the caller. Nothing changed since the last time, so the caller may 
> > again cause it to dispatch.
> > 
> > That other thread will be always in runnable state and will be at 100% CPU 
> > usage (busy-loop), waiting for this (suspended) thread to empty the pipe. The 
> > OS may take some time to wake it up from cond_wait. Meanwhile, we're burning 
> > CPU and consuming battery.
> > 
> > Unless there's something before the call to poll(2) that will prevent it from 
> > happening in the first place. I have to confess I have not yet understood this 
> > problem to confirm or deny that.
> 
> Well, the rest of read_events() is still executed:
> 
> > +		ret = poll(pfd, 2, -1);
> > +		if (pfd[1].revents & POLLIN)
> > +			read(display->reader_pipe[0], &c, 1);
> > +
> > +		pthread_mutex_lock(&display->mutex);
> > +
> > +		pthread_cond_signal(&display->pipe_cond);
> [...]
> >  	}
> >  
> > +	if (pthread_equal(display->reader, pthread_self())) {
> [...]
> > +	} else {
> > +		/* Some other thread signed up to read from the fd, so
> > +		 * just wait for the reader to do the work. */
> > +		pthread_cond_wait(&display->reader_cond, &display->mutex);
> > +	}
> > +
> > +	return 0;
> > +}
> 
> So since the LOCKED_READER already set ->reader to itself, the VOLUNTEER_READER
> will go into the else branch and sit in pthread_cond_wait() until the
> LOCKED_READER is done doings its thing. There shouldn't be a busy loop here, right?

Yeah, I think this logic is fine.

Kristian