Service Manager: Implement graceful service termination

Service Manager: Implement graceful service termination

Adds a new ServiceControl client interface associated with
a service's Service interface. This allows a service to notify
the Service Manager that it's ready to be terminated by calling
RequestQuit().

Adds an ack reply to Service.OnConnect. The Service Manager will
ignore any RequestQuit() issued while an OnConnect is still
pending acknowledgement.

Fixes a race in existing service_manager_unittests which was
causing
some tests to hang fairly regularly, using RequestQuit()
to avoid races between service shutdown and incoming connections.

Also fixes incorrect base::SimpleThread usage in some tests.

BUG=654986, 662177
Committed: https://crrev.com/5b9cb3df6c06015d79aeb688c2b8905016086b9e
Committed: https://crrev.com/66ee2e33ec55e317dfbf11209b9a4c08ecac4c46
Cr-Original-Commit-Position: refs/heads/master@{#430414}
Cr-Commit-Position: refs/heads/master@{#431354}

[Ken Rockot(use gerrit already), 2016-11-05 08:55:48]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-05 08:55:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ken Rockot(use gerrit already), 2016-11-05 08:56:00]

Description was changed from

==========
Service Mnaager: Implement graceful service termination

Adds a new ServiceControl client interface associated with
a service's Service interface. This allows a service to notify
the Service Manager that it's ready to be terminated by calling
RequestQuit().

Adds an ack reply to Service.OnConnect. The Service Manager will
ignore any RequestQuit() issued while an OnConnect is still
pending acknowledgement.

Fixes a race in existing service_manager_unittests which was
causing a some tests to hang fairly regularly, using RequestQuit()
to avoid races between shutdown and incoming connections.

Also fixes incorrect base::SimpleThread usage in some tests.

BUG=654986
==========

to

==========
Service Manager: Implement graceful service termination

Adds a new ServiceControl client interface associated with
a service's Service interface. This allows a service to notify
the Service Manager that it's ready to be terminated by calling
RequestQuit().

Adds an ack reply to Service.OnConnect. The Service Manager will
ignore any RequestQuit() issued while an OnConnect is still
pending acknowledgement.

Fixes a race in existing service_manager_unittests which was
causing a some tests to hang fairly regularly, using RequestQuit()
to avoid races between shutdown and incoming connections.

Also fixes incorrect base::SimpleThread usage in some tests.

BUG=654986
==========

[Ken Rockot(use gerrit already), 2016-11-05 08:56:47]

Description was changed from

==========
Service Manager: Implement graceful service termination

Adds a new ServiceControl client interface associated with
a service's Service interface. This allows a service to notify
the Service Manager that it's ready to be terminated by calling
RequestQuit().

Adds an ack reply to Service.OnConnect. The Service Manager will
ignore any RequestQuit() issued while an OnConnect is still
pending acknowledgement.

Fixes a race in existing service_manager_unittests which was
causing a some tests to hang fairly regularly, using RequestQuit()
to avoid races between shutdown and incoming connections.

Also fixes incorrect base::SimpleThread usage in some tests.

BUG=654986
==========

to

==========
Service Manager: Implement graceful service termination

Adds a new ServiceControl client interface associated with
a service's Service interface. This allows a service to notify
the Service Manager that it's ready to be terminated by calling
RequestQuit().

Adds an ack reply to Service.OnConnect. The Service Manager will
ignore any RequestQuit() issued while an OnConnect is still
pending acknowledgement.

Fixes a race in existing service_manager_unittests which was
causing a some tests to hang fairly regularly, using RequestQuit()
to avoid races between shutdown and incoming connections.

Also fixes incorrect base::SimpleThread usage in some tests.

BUG=654986,662177
==========

[commit-bot: I haz the power, 2016-11-05 09:22:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-05 09:22:36]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Ken Rockot(use gerrit already), 2016-11-05 15:42:21]

Description was changed from

==========
Service Manager: Implement graceful service termination

Adds a new ServiceControl client interface associated with
a service's Service interface. This allows a service to notify
the Service Manager that it's ready to be terminated by calling
RequestQuit().

Adds an ack reply to Service.OnConnect. The Service Manager will
ignore any RequestQuit() issued while an OnConnect is still
pending acknowledgement.

Fixes a race in existing service_manager_unittests which was
causing a some tests to hang fairly regularly, using RequestQuit()
to avoid races between shutdown and incoming connections.

Also fixes incorrect base::SimpleThread usage in some tests.

BUG=654986,662177
==========

to

==========
Service Manager: Implement graceful service termination

Adds a new ServiceControl client interface associated with
a service's Service interface. This allows a service to notify
the Service Manager that it's ready to be terminated by calling
RequestQuit().

Adds an ack reply to Service.OnConnect. The Service Manager will
ignore any RequestQuit() issued while an OnConnect is still
pending acknowledgement.

Fixes a race in existing service_manager_unittests which was
causing
some tests to hang fairly regularly, using RequestQuit()
to avoid races between service shutdown and incoming connections.

Also fixes incorrect base::SimpleThread usage in some tests.

BUG=654986,662177
==========

[Ken Rockot(use gerrit already), 2016-11-05 17:19:46]

rockot@chromium.org changed reviewers:
+ ben@chromium.org

[Ben Goodger (Google), 2016-11-07 21:48:12]

lgtm

[Ken Rockot(use gerrit already), 2016-11-07 21:50:33]

The CQ bit was checked by rockot@chromium.org

[commit-bot: I haz the power, 2016-11-07 21:51:09]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-07 21:58:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-07 21:58:52]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Ken Rockot(use gerrit already), 2016-11-07 22:03:27]

rockot@chromium.org changed reviewers:
+ tsepez@chromium.org

[Ken Rockot(use gerrit already), 2016-11-07 22:03:27]

+tsepez for mojom

[Tom Sepez, 2016-11-07 22:23:51]

mojom LGTM

[Ken Rockot(use gerrit already), 2016-11-07 22:40:25]

The CQ bit was checked by rockot@chromium.org

[commit-bot: I haz the power, 2016-11-07 22:40:54]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-07 23:27:53]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-11-07 23:36:08]

Description was changed from

==========
Service Manager: Implement graceful service termination

Adds a new ServiceControl client interface associated with
a service's Service interface. This allows a service to notify
the Service Manager that it's ready to be terminated by calling
RequestQuit().

Adds an ack reply to Service.OnConnect. The Service Manager will
ignore any RequestQuit() issued while an OnConnect is still
pending acknowledgement.

Fixes a race in existing service_manager_unittests which was
causing
some tests to hang fairly regularly, using RequestQuit()
to avoid races between service shutdown and incoming connections.

Also fixes incorrect base::SimpleThread usage in some tests.

BUG=654986,662177
==========

to

==========
Service Manager: Implement graceful service termination

Adds a new ServiceControl client interface associated with
a service's Service interface. This allows a service to notify
the Service Manager that it's ready to be terminated by calling
RequestQuit().

Adds an ack reply to Service.OnConnect. The Service Manager will
ignore any RequestQuit() issued while an OnConnect is still
pending acknowledgement.

Fixes a race in existing service_manager_unittests which was
causing
some tests to hang fairly regularly, using RequestQuit()
to avoid races between service shutdown and incoming connections.

Also fixes incorrect base::SimpleThread usage in some tests.

BUG=654986,662177

Committed: https://crrev.com/5b9cb3df6c06015d79aeb688c2b8905016086b9e
Cr-Commit-Position: refs/heads/master@{#430414}
==========

[commit-bot: I haz the power, 2016-11-07 23:36:09]

Patchset 1 (id:??) landed as
https://crrev.com/5b9cb3df6c06015d79aeb688c2b8905016086b9e
Cr-Commit-Position: refs/heads/master@{#430414}

[blundell, 2016-11-08 14:12:54]

blundell@chromium.org changed reviewers:
+ blundell@chromium.org

[blundell, 2016-11-08 14:12:56]

https://codereview.chromium.org/2480603004/diff/1/services/service_manager/se...
File services/service_manager/service_manager.cc (right):

https://codereview.chromium.org/2480603004/diff/1/services/service_manager/se...
services/service_manager/service_manager.cc:452: if
(!pending_service_connections_)
If there are pending service connections, should this remember the quit request
and oblige when there are no more pending connections? It seems a little strange
that the quit request gets silently dropped on the floor in this case.

[Ken Rockot(use gerrit already), 2016-11-08 14:17:40]

On Nov 8, 2016 6:12 AM, <blundell@chromium.org> wrote:
>
>
>
https://codereview.chromium.org/2480603004/diff/1/services/service_manager/se...
> File services/service_manager/service_manager.cc (right):
>
>
https://codereview.chromium.org/2480603004/diff/1/services/service_manager/se...
> services/service_manager/service_manager.cc:452: if
> (!pending_service_connections_)
> If there are pending service connections, should this remember the quit
> request and oblige when there are no more pending connections? It seems
> a little strange that the quit request gets silently dropped on the
> floor in this case.
>

No, that would be undesirable. If there were pending connections, the
service will eventually receive them. At that point it may no longer want
to quit. This behavior is well documented in the Service contact.

> https://codereview.chromium.org/2480603004/

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[blundell, 2016-11-08 14:22:58]

On 2016/11/08 14:17:40, Ken Rockot wrote:
> On Nov 8, 2016 6:12 AM, <mailto:blundell@chromium.org> wrote:
> >
> >
> >
>
https://codereview.chromium.org/2480603004/diff/1/services/service_manager/se...
> > File services/service_manager/service_manager.cc (right):
> >
> >
>
https://codereview.chromium.org/2480603004/diff/1/services/service_manager/se...
> > services/service_manager/service_manager.cc:452: if
> > (!pending_service_connections_)
> > If there are pending service connections, should this remember the quit
> > request and oblige when there are no more pending connections? It seems
> > a little strange that the quit request gets silently dropped on the
> > floor in this case.
> >
> 
> No, that would be undesirable. If there were pending connections, the
> service will eventually receive them. At that point it may no longer want
> to quit. This behavior is well documented in the Service contact.

I see, thanks.

> 
> > https://codereview.chromium.org/2480603004/
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[benwells, 2016-11-09 03:45:04]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/2486073003/ by benwells@chromium.org.

The reason for reverting is: Seems to be the cause of lock order inversions on
TSan bots.

See https://crbug.com/663557 for details.

BUG=663557.

[Ken Rockot(use gerrit already), 2016-11-10 20:09:19]

LOI has been fixed by https://codereview.chromium.org/2494483003, so I'm going
to reland this as-is.

[Ken Rockot(use gerrit already), 2016-11-10 20:09:32]

The CQ bit was checked by rockot@chromium.org

[commit-bot: I haz the power, 2016-11-10 20:09:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-10 21:16:57]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-11-10 21:23:40]

Description was changed from

==========
Service Manager: Implement graceful service termination

Adds a new ServiceControl client interface associated with
a service's Service interface. This allows a service to notify
the Service Manager that it's ready to be terminated by calling
RequestQuit().

Adds an ack reply to Service.OnConnect. The Service Manager will
ignore any RequestQuit() issued while an OnConnect is still
pending acknowledgement.

Fixes a race in existing service_manager_unittests which was
causing
some tests to hang fairly regularly, using RequestQuit()
to avoid races between service shutdown and incoming connections.

Also fixes incorrect base::SimpleThread usage in some tests.

BUG=654986,662177

Committed: https://crrev.com/5b9cb3df6c06015d79aeb688c2b8905016086b9e
Cr-Commit-Position: refs/heads/master@{#430414}
==========

to

==========
Service Manager: Implement graceful service termination

Adds a new ServiceControl client interface associated with
a service's Service interface. This allows a service to notify
the Service Manager that it's ready to be terminated by calling
RequestQuit().

Adds an ack reply to Service.OnConnect. The Service Manager will
ignore any RequestQuit() issued while an OnConnect is still
pending acknowledgement.

Fixes a race in existing service_manager_unittests which was
causing
some tests to hang fairly regularly, using RequestQuit()
to avoid races between service shutdown and incoming connections.

Also fixes incorrect base::SimpleThread usage in some tests.

BUG=654986,662177

Committed: https://crrev.com/5b9cb3df6c06015d79aeb688c2b8905016086b9e
Committed: https://crrev.com/66ee2e33ec55e317dfbf11209b9a4c08ecac4c46
Cr-Original-Commit-Position: refs/heads/master@{#430414}
Cr-Commit-Position: refs/heads/master@{#431354}
==========

[commit-bot: I haz the power, 2016-11-10 21:23:41]

Patchset 1 (id:??) landed as
https://crrev.com/66ee2e33ec55e317dfbf11209b9a4c08ecac4c46
Cr-Commit-Position: refs/heads/master@{#431354}