AU: Fix potential issues with premature destruction of HTTP fetchers.
This patch adds a new TransferTerminated callback to the
HttpFetcher class. It fixes two potential memory corruption
issues with premature destruction of HttpFetcher instances:
1. When MultiHttpFetcher completes a range, it terminates
the current fetcher and starts the next one, if any. Change
so that the next fetcher is started when the
TransferTerminated callback is received from the current
fetcher. This prevents the multi fetcher from sending a
TransferComplete/TransferTerminated callbacks before the
underlying fetcher is cleaned up, which may lead to the
fetchers being destroyed prematurely.
2. If the download action fails due to a failed write,
terminate the transfer and then wait for the transfer
terminated callback before notifying the action processor
that the action is complete. Otherwise, the action may get
destroyed before the transfer is actually terminated
possibly leading to memory corruption, etc.
Hopefully these changes fix crosbug.com/8798.
BUG=8798
TEST=unit tests, tested on device with write errors
Change-Id: If416b95625ab31662f2e1308df6bdd1757a2ad78
Review URL: http://codereview.chromium.org/5009009
diff --git a/http_fetcher.h b/http_fetcher.h
index d0f9cb2..6d8608b 100644
--- a/http_fetcher.h
+++ b/http_fetcher.h
@@ -45,11 +45,13 @@
// Downloading should resume from this offset
virtual void SetOffset(off_t offset) = 0;
- // Begins the transfer to the specified URL.
+ // Begins the transfer to the specified URL. This fetcher instance should not
+ // be destroyed until either TransferComplete, or TransferTerminated is
+ // called.
virtual void BeginTransfer(const std::string& url) = 0;
- // Aborts the transfer. TransferComplete() will not be called on the
- // delegate.
+ // Aborts the transfer. The transfer may not abort right away -- delegate's
+ // TransferTerminated() will be called when the transfer is actually done.
virtual void TerminateTransfer() = 0;
// If data is coming in too quickly, you can call Pause() to pause the
@@ -97,9 +99,14 @@
// Called if the fetcher seeks to a particular offset.
virtual void SeekToOffset(off_t offset) {}
- // Called when the transfer has completed successfully or been somehow
- // aborted.
+ // Called when the transfer has completed successfully or been aborted through
+ // means other than TerminateTransfer. It's OK to destroy the |fetcher| object
+ // in this callback.
virtual void TransferComplete(HttpFetcher* fetcher, bool successful) = 0;
+
+ // Called when the transfer has been aborted through TerminateTransfer. It's
+ // OK to destroy the |fetcher| object in this callback.
+ virtual void TransferTerminated(HttpFetcher* fetcher) {}
};
} // namespace chromeos_update_engine