Bug 1347657 - Use array entry as value instead of reference to avoid being invalidated by realloc. draft
authorHenry Chang <hchang@mozilla.com>
Thu, 16 Mar 2017 15:19:45 +0800
changeset 499807 1703b9ea7902b35669e867d6fd61a67e579fc4dc
parent 499512 ff04d410e74b69acfab17ef7e73e7397602d5a68
child 549469 ec3237fe2ab23733ebc0e3342b79680e7ffb46b6
push id49537
push userhchang@mozilla.com
push dateThu, 16 Mar 2017 07:20:37 +0000
bugs1347657
milestone55.0a1
Bug 1347657 - Use array entry as value instead of reference to avoid being invalidated by realloc. nsTArray::AppendElement may cause memory reallocation if out of capacity. In nsUrlClassifierStreamUpdater::FetchNextRequest(), we take the reference of the first element of mPendingRequests and pass its member as reference to DownloadUpdate(), where mPendingRequests.AppendElement will be called. If the AppendElement in DownloadUpdate() causes realloc, the reference becomes dangling. The most efficient fix is to "move" the reference's (i.e. request) member variables to DownloadUpdate() but I think in this case we can just take the value from the array and pass it around with no given that the array element contains simply a couple of strings and pointers. MozReview-Commit-ID: KEZ5d3l3HoI
toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.cpp
--- a/toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.cpp
@@ -370,34 +370,30 @@ nsUrlClassifierStreamUpdater::FetchNext(
 nsresult
 nsUrlClassifierStreamUpdater::FetchNextRequest()
 {
   if (mPendingRequests.Length() == 0) {
     LOG(("No more requests, returning"));
     return NS_OK;
   }
 
-  PendingRequest &request = mPendingRequests[0];
+  PendingRequest request = mPendingRequests[0];
+  mPendingRequests.RemoveElementAt(0);
   LOG(("Stream updater: fetching next request: %s, %s",
        request.mTables.get(), request.mUrl.get()));
   bool dummy;
   DownloadUpdates(
     request.mTables,
     request.mRequestPayload,
     request.mIsPostRequest,
     request.mUrl,
     request.mSuccessCallback,
     request.mUpdateErrorCallback,
     request.mDownloadErrorCallback,
     &dummy);
-  request.mSuccessCallback = nullptr;
-  request.mUpdateErrorCallback = nullptr;
-  request.mDownloadErrorCallback = nullptr;
-  mPendingRequests.RemoveElementAt(0);
-
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsUrlClassifierStreamUpdater::StreamFinished(nsresult status,
                                              uint32_t requestedDelay)
 {
   // We are a service and may not be reset with Init between calls, so reset