AU: Fix potential issues with premature destruction of HTTP fetchers.

download_action.cc

 // Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "update_engine/download_action.h"
 #include <errno.h>
 #include <algorithm>
 #include <string>
 #include <vector>
 #include <glib.h>
 #include "update_engine/action_pipe.h"
 #include "update_engine/subprocess.h"
 
 using std::min;
 using std::string;
 using std::vector;
 
 namespace chromeos_update_engine {
 
 // Use a buffer to reduce the number of IOPS on SSD devices.
 const size_t kFileWriterBufferSize = 128 * 1024;  // 128 KiB
 
 DownloadAction::DownloadAction(PrefsInterface* prefs,
                                HttpFetcher* http_fetcher)
     : prefs_(prefs),
       writer_(NULL),
       http_fetcher_(http_fetcher),
+      code_(kActionCodeSuccess),
       delegate_(NULL),
       bytes_received_(0) {}
 
 DownloadAction::~DownloadAction() {}
 
 void DownloadAction::PerformAction() {
   http_fetcher_->set_delegate(this);
 
   // Get the InstallPlan and read it
   CHECK(HasInputObject());
@@ skipping 56 matching lines @@
     delegate_->SetDownloadStatus(true);  // Set to active.
   }
   http_fetcher_->BeginTransfer(install_plan_.download_url);
 }
 
 void DownloadAction::TerminateProcessing() {
   if (writer_) {
     LOG_IF(WARNING, writer_->Close() != 0) << "Error closing the writer.";
     writer_ = NULL;
   }
-  http_fetcher_->TerminateTransfer();
   if (delegate_) {
     delegate_->SetDownloadStatus(false);  // Set to inactive.
   }
+  // Terminates the transfer. The action is terminated, if necessary, when the
+  // TransferTerminated callback is received.
+  http_fetcher_->TerminateTransfer();
 }
 
 void DownloadAction::SeekToOffset(off_t offset) {
   bytes_received_ = offset;
 }
 
 void DownloadAction::ReceivedBytes(HttpFetcher *fetcher,
                                    const char* bytes,
                                    int length) {
   bytes_received_ += length;
   if (delegate_)
     delegate_->BytesReceived(bytes_received_, install_plan_.size);
   if (writer_ && writer_->Write(bytes, length) < 0) {
     LOG(ERROR) << "Write error -- terminating processing.";
+    // Don't tell the action processor that the action is complete until we get
+    // the TransferTerminated callback. Otherwise, this and the HTTP fetcher
+    // objects may get destroyed before all callbacks are complete.
+    code_ = kActionCodeDownloadWriteError;
     TerminateProcessing();
-    processor_->ActionComplete(this, kActionCodeDownloadWriteError);
     return;
   }
   // DeltaPerformer checks the hashes for delta updates.
   if (install_plan_.is_full_update) {
     omaha_hash_calculator_.Update(bytes, length);
   }
 }
 
 namespace {
 void FlushLinuxCaches() {
@@ skipping 53 matching lines @@
   }
 
   FlushLinuxCaches();
 
   // Write the path to the output pipe if we're successful.
   if (code == kActionCodeSuccess && HasOutputPipe())
     SetOutputObject(GetInputObject());
   processor_->ActionComplete(this, code);
 }
 
+void DownloadAction::TransferTerminated(HttpFetcher *fetcher) {
+  if (code_ != kActionCodeSuccess) {
+    processor_->ActionComplete(this, code_);
+  }
+}
+
 };  // namespace {}