The following 3 patches changes the way services are started up and identify themselves to the monitor.
The first patch is just minor or cosmetic fixes.
The second patch changes the identification from monitor initiated to service initiated and acked by the monitor. This allows the service to perform the identification after it has completely set up all the dbus channel and is ready to receive a reply avoiding nasty race conditions.
The third patch changes the way services are started at startup. The Data Provider service is given a 1 second advantage so that it should normally be all set up by the time other service start and try to connect to it. This way services normally avoid racing against dp and can fully initialize on the first attempt and not rely on reconnection.
Initial testing seem to work pretty well and startup does not show ugly errors anymore.
I may send an additional separate patch to make services identify against dp the same way they do against the monitor (ie service initiated instead of server initiated). A patch will follow if I can pull it off in a short time frame.
Simo.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 08/11/2009 06:52 AM, Simo Sorce wrote:
The following 3 patches changes the way services are started up and identify themselves to the monitor.
The first patch is just minor or cosmetic fixes.
The second patch changes the identification from monitor initiated to service initiated and acked by the monitor. This allows the service to perform the identification after it has completely set up all the dbus channel and is ready to receive a reply avoiding nasty race conditions.
The third patch changes the way services are started at startup. The Data Provider service is given a 1 second advantage so that it should normally be all set up by the time other service start and try to connect to it. This way services normally avoid racing against dp and can fully initialize on the first attempt and not rely on reconnection.
Initial testing seem to work pretty well and startup does not show ugly errors anymore.
I may send an additional separate patch to make services identify against dp the same way they do against the monitor (ie service initiated instead of server initiated). A patch will follow if I can pull it off in a short time frame.
Simo.
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
Patch 0001: Ack Patch 0002: Minor typo in client_registration: regeistration should be registration Coding standard for dbus methods suggest the more readable: ret = dbus_message_append_args(reply, DBUS_TYPE_STRING, &version, DBUS_TYPE_INVALID); Introduces a segmentation fault when an LDAP server is very slow. The ping timeout is hit and frees the connection, but it subsequently segfaults on a double-free.
I'd rather that we moved the monitor versions to integers. We never actually used the dbus_get_monitor_version() call previously, so there's no ABI break if we simply move the version to 1 now.
600000 seconds is way too long a timeout for monitor_common_send_id. Prefer -1 (which libdbus defines as "a reasonable timeout") if you don't know what to set.
Patch 0003: Ack.
- -- Stephen Gallagher RHCE 804006346421761
Looking to carve out IT costs? www.redhat.com/carveoutcosts/
On Tue, 2009-08-11 at 08:41 -0400, Stephen Gallagher wrote:
Patch 0002: Minor typo in client_registration: regeistration should be registration
ok
Coding standard for dbus methods suggest the more readable: ret = dbus_message_append_args(reply, DBUS_TYPE_STRING, &version, DBUS_TYPE_INVALID);
ok
Introduces a segmentation fault when an LDAP server is very slow. The ping timeout is hit and frees the connection, but it subsequently segfaults on a double-free.
I added a spy to the connection structure so that the svc reference is removed when it goes away, this should fix the segfault.
I'd rather that we moved the monitor versions to integers. We never actually used the dbus_get_monitor_version() call previously, so there's no ABI break if we simply move the version to 1 now.
the change in this patch is already a huge internal ABI and API break anyway :-) But it doesn't matter until we reach v 1.0 and start seeing people building external backends. Changed monitor version to be a hex number as other ones.
600000 seconds is way too long a timeout for monitor_common_send_id. Prefer -1 (which libdbus defines as "a reasonable timeout") if you don't know what to set.
That's 600k milliseconds IIRC, not changing it as I simply reused what was there before. We will have a session to review all timeouts and set proper values once and for all.
Attached new 0002 patch with the included fixes.
Simo.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 08/11/2009 12:08 PM, Simo Sorce wrote:
Attached new 0002 patch with the included fixes.
Ooops patch was buggy, sent int tried to read string (thanks Steve :)
New one attached.
Simo.
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
Ack.
- -- Stephen Gallagher RHCE 804006346421761
Looking to carve out IT costs? www.redhat.com/carveoutcosts/
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 08/11/2009 12:18 PM, Stephen Gallagher wrote:
On 08/11/2009 12:08 PM, Simo Sorce wrote:
Attached new 0002 patch with the included fixes.
Ooops patch was buggy, sent int tried to read string (thanks Steve :)
New one attached.
Simo.
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
Ack.
Pushed to master. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
- -- Stephen Gallagher RHCE 804006346421761
Looking to carve out IT costs? www.redhat.com/carveoutcosts/
sssd-devel@lists.fedorahosted.org
-
Simo Sorce -
Stephen Gallagher