[PATCH] fix DBusWatch handling

[Simo Sorce]

After some playing around with Stephen patch and adding some debugging I
finally understood how DBUS likes to deal with watches.

We erroneously assumed that DBUS would add only one watch per FD.
It doesn't, instead it always adds 2 watches.
One to catch read events and one to catch write events.

It then proceed to always keep one of the 2 disabled when the other is
enabled, and it always toggles both when it needs to reverse access to
the file descriptor.

The following patch keeps track of sbus-watches/fd pairs and allocate
both DBusWatches to the same structure.

So far testing seem to confirm this approach works as expected.

Simo.

[Simo Sorce]

On Thu, 2009-08-27 at 00:35 -0400, Simo Sorce wrote:
> After some playing around with Stephen patch and adding some debugging I
> finally understood how DBUS likes to deal with watches.
>
> We erroneously assumed that DBUS would add only one watch per FD.
> It doesn't, instead it always adds 2 watches.
> One to catch read events and one to catch write events.
>
> It then proceed to always keep one of the 2 disabled when the other is
> enabled, and it always toggles both when it needs to reverse access to
> the file descriptor.
>
> The following patch keeps track of sbus-watches/fd pairs and allocate
> both DBusWatches to the same structure.
>
> So far testing seem to confirm this approach works as expected.

New revision.

I was finally able to test this patch on an Ubuntu machine (thanks
Steve) and we found out 2 pretty nasty bugs masquerading each other on
Fedora.

This new revision should be final, it works on Ubuntu and on Fedora.

Simo.

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/31/2009 02:47 PM, Simo Sorce wrote:
> On Thu, 2009-08-27 at 00:35 -0400, Simo Sorce wrote:
> > After some playing around with Stephen patch and adding some debugging I
> > finally understood how DBUS likes to deal with watches.
> >
> > We erroneously assumed that DBUS would add only one watch per FD.
> > It doesn't, instead it always adds 2 watches.
> > One to catch read events and one to catch write events.
> >
> > It then proceed to always keep one of the 2 disabled when the other is
> > enabled, and it always toggles both when it needs to reverse access to
> > the file descriptor.
> >
> > The following patch keeps track of sbus-watches/fd pairs and allocate
> > both DBusWatches to the same structure.
> >
> > So far testing seem to confirm this approach works as expected.
>
> New revision.
>
> I was finally able to test this patch on an Ubuntu machine (thanks
> Steve) and we found out 2 pretty nasty bugs masquerading each other on
> Fedora.
>
> This new revision should be final, it works on Ubuntu and on Fedora.
>
> Simo.
>
> sssd-devel mailing list
> [e-mail]
> https://fedorahosted.org/mailman/listinfo/sssd-devel

Ack and pushed to master.

- -- 
Stephen Gallagher
RHCE 804006346421761

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkqcICcACgkQeiVVYja6o6PT5QCgnXsS6aeKeOgov1BpMTLiE9Qz
> bpQAn2R/oG62agKgRx3Cxq8WwSWimsEc
> =/qv8
> -----END PGP SIGNATURE-----

[Mathias Gug]

Hi,

On Mon, Aug 31, 2009 at 3:10 PM, Stephen Gallagher  wrote:
> > New revision.
> >
> > I was finally able to test this patch on an Ubuntu machine (thanks
> > Steve) and we found out 2 pretty nasty bugs masquerading each other on
> > Fedora.
> >
> > This new revision should be final, it works on Ubuntu and on Fedora.
>
> Ack and pushed to master.

I've uploaded a new version of sssd in Karmic (0.5.0-0ubuntu2) that
includes this patch. Thanks again for chasing this down!