Commit 82a08f28 authored by weitaosu's avatar weitaosu Committed by Commit bot

Stop the daemon process from repeatedly starting up and shutting down the...

Stop the daemon process from repeatedly starting up and shutting down the network process when the host has been unregistered.

We have two bugs in the chromoting host that caused the repeated startup and shutdown of the network process when the host has be unregistered:
1. The host process never exits naturally. As a result, the WorkerProcessLauncher in the daemon process never receives the exit code and thus isn't aware that a "permanent" error occured.
2. If a host has service account enabled, it gets kInvalidOauthCredentialsExitCode instead of kInvalidHostIdExitCode if the host has been unregistered in the service directory. But we only disable the service upon the latter.

Issue 1 is a regression from https://codereview.chromium.org/891663005 which added a dangling reference to the UrlRequestContextGetterin HostProcessMain. This leaked object will also keep the Network task runner, and thus the UI task runner alive. So the message loop in HostProcessMain never quits.

I created crbug.com/475213 to track the original DCHECK.

BUG=472884

Review URL: https://codereview.chromium.org/1061903002

Cr-Commit-Position: refs/heads/master@{#324377}
parent 99ed2aab
...@@ -21,6 +21,7 @@ ...@@ -21,6 +21,7 @@
#include "remoting/host/config_file_watcher.h" #include "remoting/host/config_file_watcher.h"
#include "remoting/host/desktop_session.h" #include "remoting/host/desktop_session.h"
#include "remoting/host/host_event_logger.h" #include "remoting/host/host_event_logger.h"
#include "remoting/host/host_exit_codes.h"
#include "remoting/host/host_status_observer.h" #include "remoting/host/host_status_observer.h"
#include "remoting/host/screen_resolution.h" #include "remoting/host/screen_resolution.h"
#include "remoting/protocol/transport.h" #include "remoting/protocol/transport.h"
...@@ -132,6 +133,9 @@ bool DaemonProcess::OnMessageReceived(const IPC::Message& message) { ...@@ -132,6 +133,9 @@ bool DaemonProcess::OnMessageReceived(const IPC::Message& message) {
void DaemonProcess::OnPermanentError(int exit_code) { void DaemonProcess::OnPermanentError(int exit_code) {
DCHECK(caller_task_runner()->BelongsToCurrentThread()); DCHECK(caller_task_runner()->BelongsToCurrentThread());
DCHECK(kMinPermanentErrorExitCode <= exit_code &&
exit_code <= kMaxPermanentErrorExitCode);
Stop(); Stop();
} }
......
...@@ -140,11 +140,17 @@ void DaemonProcessWin::OnChannelConnected(int32 peer_pid) { ...@@ -140,11 +140,17 @@ void DaemonProcessWin::OnChannelConnected(int32 peer_pid) {
} }
void DaemonProcessWin::OnPermanentError(int exit_code) { void DaemonProcessWin::OnPermanentError(int exit_code) {
// Change the service start type to 'manual' if the host has been deleted DCHECK(kMinPermanentErrorExitCode <= exit_code &&
// remotely. This way the host will not be started every time the machine exit_code <= kMaxPermanentErrorExitCode);
// boots until the user re-enable it again.
if (exit_code == kInvalidHostIdExitCode) // Both kInvalidHostIdExitCode and kInvalidOauthCredentialsExitCode are
// errors then will never go away with the current config.
// Disabling automatic service start until the host is re-enabled and config
// updated.
if (exit_code == kInvalidHostIdExitCode ||
exit_code == kInvalidOauthCredentialsExitCode) {
DisableAutoStart(); DisableAutoStart();
}
DaemonProcess::OnPermanentError(exit_code); DaemonProcess::OnPermanentError(exit_code);
} }
......
...@@ -1545,13 +1545,6 @@ int HostProcessMain() { ...@@ -1545,13 +1545,6 @@ int HostProcessMain() {
scoped_ptr<net::NetworkChangeNotifier> network_change_notifier( scoped_ptr<net::NetworkChangeNotifier> network_change_notifier(
net::NetworkChangeNotifier::Create()); net::NetworkChangeNotifier::Create());
// BasicURLRequestContext holds references to threads, so it needs to be
// dereferences on UI threads. Store the reference to the URLRequestGetter to
// make sure it's not destroyed on other threads.
// TODO(sergeyu): Consider fixing it in BasicURLRequestContext.
scoped_refptr<net::URLRequestContextGetter> url_request_context_getter =
context->url_request_context_getter();
// Create & start the HostProcess using these threads. // Create & start the HostProcess using these threads.
// TODO(wez): The HostProcess holds a reference to itself until Shutdown(). // TODO(wez): The HostProcess holds a reference to itself until Shutdown().
// Remove this hack as part of the multi-process refactoring. // Remove this hack as part of the multi-process refactoring.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment