fix race on kcrash auto-restarts
ClosedPublic

Authored by sitter on Mar 27 2019, 12:52 PM.

Details

Summary

When kcrash is set to auto restart Unique service processes we have
potential for a race. Because KCrash will want to ideally restart through
kdeinit it only closes the crashed process' sockets after it issues
the restart. Depending on timing the new process may then attempt to
register its service name while the crashed process is still holding the
name. As a result the new process will not actually start because it'll
fail to claim the name.

To mitigate this problem, service registration now will wait for a bit
iff the service name cannot be registered and it cannot successfully hand
over to the registered instance (e.g. the instance is defunct for whatever
reason and not responding to dbus calls).

Alas, this continues to have some race potential. The problem ultimately
is that we do not know why the service name is already registered and also
cannot ask the service since its not responding to calls. To further
restrict the potential of a race the wait time is increased substantially
when the environment variable KCRASH_AUTO_RESTARTED is set.

Test Plan

builds, test passes, doesn't break existing apps

Diff Detail

Repository
R271 KDBusAddons
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
sitter created this revision.Mar 27 2019, 12:52 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptMar 27 2019, 12:52 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
sitter requested review of this revision.Mar 27 2019, 12:52 PM

Still fairly messy and very verbose but working.

@broulik @davidedmundson I'd love some thoughts on this.

Also, @davidedmundson since you suggested the registration queuing. I do wonder if that isn't actually posing a risk. If the service successfully registers between bus->registerService(..) and the fallback call iface.Activate(platform_data);, doesn't the process run risk of possibly talking to itself? ...and because of the exit() ending up terminating itself.

++++++

Needs some cleanup, but the logic is all sound.

sitter updated this revision to Diff 62840.Jul 31 2019, 11:04 AM

clean up a bit

sitter retitled this revision from RFC fix race on kcrash auto-restarts to fix race on kcrash auto-restarts.Jul 31 2019, 11:05 AM
sitter edited the test plan for this revision. (Show Details)
davidedmundson accepted this revision.Jul 31 2019, 11:40 AM

Don't ship till after tagging though, better to have a minor bug we know than risk something worse.

This revision is now accepted and ready to land.Jul 31 2019, 11:40 AM

Yeah, was thinking the same thing :)

This revision was automatically updated to reflect the committed changes.