HostResolverSystemTask::StartLookupAttempt can delete itself synchron…

…ously

When a system dns resolver override exists, it may call
OnLookupComplete() synchronously, resulting in deleting `this`. It
should not access any member fields after calling the override.

Bug: 1449497
Change-Id: If53ce33597591ba2e2761eb38cb13ae08e71d2f4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4570328
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1150217}

net/dns/host_resolver_system_task.cc

@@ -273,23 +273,6 @@ void HostResolverSystemTask::StartLookupAttempt() {
   DCHECK(!was_completed());
   ++attempt_number_;
 
-  auto lookup_complete_cb =
-      base::BindOnce(&HostResolverSystemTask::OnLookupComplete,
-                     weak_ptr_factory_.GetWeakPtr(), attempt_number_);
-
-  // If a hook has been installed, call it instead of posting a resolution task
-  // to a worker thread.
-  if (GetSystemDnsResolverOverride()) {
-    GetSystemDnsResolverOverride().Run(hostname_, address_family_, flags_,
-                                       std::move(lookup_complete_cb), network_);
-  } else {
-    base::OnceCallback<int(AddressList * addrlist, int* os_error)> resolve_cb =
-        base::BindOnce(&ResolveOnWorkerThread, params_.resolver_proc, hostname_,
-                       address_family_, flags_, network_);
-    PostSystemDnsResolutionTaskAndReply(std::move(resolve_cb),
-                                        std::move(lookup_complete_cb));
-  }
-
   net_log_.AddEventWithIntParams(
       NetLogEventType::HOST_RESOLVER_MANAGER_ATTEMPT_STARTED, "attempt_number",
       attempt_number_);
@@ -307,6 +290,25 @@ void HostResolverSystemTask::StartLookupAttempt() {
         params_.unresponsive_delay *
             std::pow(params_.retry_factor, attempt_number_ - 1));
   }
+
+  auto lookup_complete_cb =
+      base::BindOnce(&HostResolverSystemTask::OnLookupComplete,
+                     weak_ptr_factory_.GetWeakPtr(), attempt_number_);
+
+  // If a hook has been installed, call it instead of posting a resolution task
+  // to a worker thread.
+  if (GetSystemDnsResolverOverride()) {
+    GetSystemDnsResolverOverride().Run(hostname_, address_family_, flags_,
+                                       std::move(lookup_complete_cb), network_);
+    // Do not add code below. `lookup_complete_cb` may have already deleted
+    // `this`.
+  } else {
+    base::OnceCallback<int(AddressList * addrlist, int* os_error)> resolve_cb =
+        base::BindOnce(&ResolveOnWorkerThread, params_.resolver_proc, hostname_,
+                       address_family_, flags_, network_);
+    PostSystemDnsResolutionTaskAndReply(std::move(resolve_cb),
+                                        std::move(lookup_complete_cb));
+  }
 }
 
 // Callback for when DoLookup() completes.