[libvirt] 'make check' hangs

[Jiri Denemark]

On Wed, Nov 30, 2011 at 11:54:55 +0000, Daniel P. Berrange wrote:
> On Wed, Nov 30, 2011 at 11:48:16AM +0100, Jiri Denemark wrote:
> static bool
> vshDeinit(vshControl *ctl)
> {
>     ctl->quit = true;
>     vshReadlineDeinit(ctl);
>     vshCloseLogFile(ctl);
>     VIR_FREE(ctl->name);
>     if (ctl->conn) {
>         int ret;
>         if ((ret = virConnectClose(ctl->conn)) != 0) {
>             vshError(ctl, _("Failed to disconnect from the hypervisor, %d leaked reference(s)"), ret);
>         }
>     }
>     virResetLastError();
> 
>     if (ctl->eventLoopStarted) {
>         /* HACK: Add a dummy timeout to break event loop */
>         int timer = virEventAddTimeout(-1, NULL, NULL, NULL);
> 
> 
> Now look at the event loop thread:
> 
> static void
> vshEventLoop(void *opaque)
> {
>     vshControl *ctl = opaque;
> 
>     while (!ctl->quit) {
>         if (virEventRunDefaultImpl() < 0) {
>             virshReportError(ctl);
>         }
>     }
> }
> 
> Neither the read of ctl->quit, nor the write of ctl->quit are protected
> by any locking. While you have assigned to ctl->quit before adding the
> dummy function, I'm not convinced that is sufficient, because in the
> absence of any thread synchronization point, the compiler is free to
> reorder your assignment to occur later. In addition, the data is not
> neccessarily coherant across threads. IMHO, the read/write of ctl->quit
> needs to be protected by a mutex.

Yeah, you're right. I seem to keep forgetting about this memory barrier "side
effect" of mutexes, which in fact is the only thing we need here :-)

Jirka