Index: [thread] [date] [subject] [author]
  From: Marcus Sundberg <mackan@stacken.kth.se>
  To  : ggi-develop@eskimo.com
  Date: Tue, 13 Apr 1999 15:04:09 +0000

Re: Multithreaded Event Queue Followup

Graydon Hoare wrote:
> 
> Allo. Andy asked me to give y'all the lowdown on the threading bug in
> gii events and I figured I'd take the opportunity to suggest a
> fix. tell me I am stupid and know not what I'm doing if you like --
> it's an important problem and I'd just like to see it fixed asap,
> preferably by someone who knows the code rather than (ahem) myself.
> 
> here's code which exhibits the problem
> 
> Thread A (in a loop):
> ggi_event_mask mask = ggi_event_mask (emCommand | emKeyboard
>         | emPtrMove | emPtrButtonPress | emPtrButtonRelease);
> ggi_event *event = new ggi_event;
> ggiEventPoll(visual, mask, NULL);
> ggiEventRead(visual, event, mask);

Just FYI - ggiEventRead() will block until there is a suitable event
available,
so the ggiEventPoll() is not necessary in the above code.

> // we are being woken up by the damage subsystem
> if (event->any.origin == GII_EV_ORIGIN_SENDEVENT) return;
> 
> Thread B (an upcall from a forced redraw, wanting to interrupt the
> event reader and take over GGI for drawing):
> 
> ggi_event *damageEvent = new ggi_event;
> gettimeofday(&(damageEvent->any.time), NULL);

Setting the time field has no effect as events are timestamped by
EventSend.

> damageEvent->any.type = evCommand;

You must also set the size and target fields.
size should be sizeof(gii_cmd_nodata_event), and target should be
GII_EV_TARGET_QUEUE (yes this does not exist yet, but it will soon ;)

> ggiEventSend (visual, damageEvent);
> 
> Now, the problem is that while this works on the first pass (it wakes
> up thread A), when thread A finally returns to read another normal
> event, the queue structure has been borked. I read in your list
> archives that marcus (a) has confirmed this and (b) has a way to fix
> it in the case of non-blocked threads so it doesn't corrupt the event
> queue structure. However, you still have a problem -- my
> ggiEventSend() is going to block if you put a mutex on the queue,
> since the polling thread will have obtained this mutex. To solve it,
> there are 2 options which should work equally well, and I leave it to
> the implementor's sense of taste:
> 
> #1:
> make local socket file descriptor inside the library for each
> queue. on poll with NULL timeout, obtain a queue mutex, check queue
> for contents, release mutex, enter select() with special file
> descriptor in the select set;
> 
> on eventSend, obtain mutex, insert event, release mutex, write a byte
> on the socket file descriptor.
> 
> thread in select wakes up, do FD_ISSET() and if it was the socket file
> descriptor, return immediately, else follow normal copy-event-to-queue
> procedure.
> 
> #2:
> construct new thread within the library which sits in select() loop
> feeding all event queues. only lock queues for an instant, during an
> insert or a remove.
> 
> make a pthreads condition variable on each queue, and when someone
> polls an empty queue, put them to sleep on the condition.
> 
> whenever anything inserts an event, from eventSend() or from select(),
> do a notify() on the condition.
> 
> #1 could introduce a number of new file descriptors and involves more
> system calls, #2 is somewhat more convoluted and has a new thread
> running around in your library.

Yes, #1 is what I had in mind. #2 is out of the question due to the
kludgy and broken way pthreads are implemented on most UNIX variants.
You simply can't use pthread functions in a library if you want the
library to work with non-threaded applications. Because of this we are
even going to implement our own mutexes for the next beta.

//Marcus
-- 
-------------------------------+------------------------------------
        Marcus Sundberg        | http://www.stacken.kth.se/~mackan/
 Royal Institute of Technology |       Phone: +46 707 295404
       Stockholm, Sweden       |   E-Mail: mackan@stacken.kth.se

Index: [thread] [date] [subject] [author]