Skip to content

Commit

Permalink
Update engine reinitializes the dbus connection and retries upon error.
Browse files Browse the repository at this point in the history
Before this fix, update engine would keep using a failing dbus object
that was initialized once during process startup.  This causes it to
fail retrieving HTTP proxy information from Chrome in certain OOBE
scenarios, as well as when switching into / out of guest mode (see
issues 25901 and 26077). With this fix, update engine reinitializes the
dbus proxy object when dbus calls using this proxy fail with a null
error pointer.  Although these circumstances may indicate that there's
a deeper problem inside dbus, this workaround seems like a good, safe
practice, and appears to be fixing the problem.

BUG=chromium-os:25901, chromium-os:26077
TEST=Passes update_engine unittests; running the image with fluctuating
guest mode triggers reinitialization, which fixes the problem.

Change-Id: I19e81b6b718da59e22f388b264f9a723c2858a1a
Reviewed-on: https://gerrit.chromium.org/gerrit/15693
Reviewed-by: Andrew de los Reyes <[email protected]>
Reviewed-by: Don Garrett <[email protected]>
Commit-Ready: Gilad Arnold <[email protected]>
Tested-by: Gilad Arnold <[email protected]>
  • Loading branch information
Gilad Arnold authored and Gerrit committed Feb 10, 2012
1 parent 34bf1ee commit 1877c39
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 15 deletions.
74 changes: 59 additions & 15 deletions chrome_browser_proxy_resolver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -104,17 +104,24 @@ bool ChromeBrowserProxyResolver::Init() {
return true;
}

void ChromeBrowserProxyResolver::Shutdown() {
if (!proxy_)
return;

GError* gerror = NULL;
DBusGConnection* gbus = dbus_->BusGet(DBUS_BUS_SYSTEM, &gerror);
TEST_AND_RETURN(gbus);
DBusConnection* connection = dbus_->ConnectionGetConnection(gbus);
dbus_->DbusConnectionRemoveFilter(
connection,
&ChromeBrowserProxyResolver::StaticFilterMessage,
this);
proxy_ = NULL;
}

ChromeBrowserProxyResolver::~ChromeBrowserProxyResolver() {
if (proxy_) {
GError* gerror = NULL;
DBusGConnection* gbus = dbus_->BusGet(DBUS_BUS_SYSTEM, &gerror);
TEST_AND_RETURN(gbus);
DBusConnection* connection = dbus_->ConnectionGetConnection(gbus);
dbus_->DbusConnectionRemoveFilter(
connection,
&ChromeBrowserProxyResolver::StaticFilterMessage,
this);
}
// Kill proxy object.
Shutdown();
// Kill outstanding timers
for (TimeoutsMap::iterator it = timers_.begin(), e = timers_.end(); it != e;
++it) {
Expand All @@ -128,19 +135,56 @@ bool ChromeBrowserProxyResolver::GetProxiesForUrl(const string& url,
void* data) {
GError* error = NULL;
guint timeout = timeout_;
if (!proxy_ || !dbus_->ProxyCall(
if (proxy_) {
bool dbus_success = true;
bool dbus_reinit = false;
bool dbus_got_error;

do {
if (!dbus_success) {
// We failed with a null error, time to re-init the dbus proxy.
LOG(WARNING) << "attempting to reinitialize dbus proxy and retrying";
Shutdown();
if (!Init()) {
LOG(WARNING) << "failed to reinitialize the dbus proxy";
break;
}
dbus_reinit = true;
}

dbus_success = dbus_->ProxyCall(
proxy_,
kLibCrosServiceResolveNetworkProxyMethodName,
&error,
G_TYPE_STRING, url.c_str(),
G_TYPE_STRING, kLibCrosProxyResolveSignalInterface,
G_TYPE_STRING, kLibCrosProxyResolveName,
G_TYPE_INVALID, G_TYPE_INVALID)) {
LOG(WARNING) << "dbus_g_proxy_call failed: "
<< utils::GetAndFreeGError(&error)
<< " Continuing with no proxy.";
G_TYPE_INVALID, G_TYPE_INVALID);

dbus_got_error = false;

if (dbus_success) {
LOG(INFO) << "dbug_g_proxy_call succeeded!";
} else {
if (error) {
// Register the fact that we did receive an error, as it is nullified
// on the next line.
dbus_got_error = true;
LOG(WARNING) << "dbus_g_proxy_call failed: "
<< utils::GetAndFreeGError(&error)
<< " Continuing with no proxy.";
} else {
LOG(WARNING) << "dbug_g_proxy_call failed with no error string, "
"continuing with no proxy.";
}
timeout = 0;
}
} while (!(dbus_success || dbus_got_error || dbus_reinit));
} else {
LOG(WARNING) << "dbug proxy object missing, continuing with no proxy.";
timeout = 0;
}

callbacks_.insert(make_pair(url, make_pair(callback, data)));
Closure* closure = NewCallback(this,
&ChromeBrowserProxyResolver::HandleTimeout,
Expand Down
3 changes: 3 additions & 0 deletions chrome_browser_proxy_resolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ class ChromeBrowserProxyResolver : public ProxyResolver {
bool delete_timer,
std::pair<ProxiesResolvedFn, void*>* callback);

// Shutdown the dbus proxy object.
void Shutdown();

DbusGlibInterface* dbus_;
DBusGProxy* proxy_;
DBusGProxy* peer_proxy_;
Expand Down

0 comments on commit 1877c39

Please sign in to comment.