Fix race condition caused by fd reused for DnsResolver
There might be a gap between fd close and fd event listener unregister.
If fd is reused for another query during that gap, it might cause the
query failed with no response since addOnFileDescriptorEventListener
method failed. To fix this problem, we must ensure that fd event
listener is unregistered before fd closing.
Bug: 134310704
Test: atest DnsResolverTest
Change-Id: Ifb6bd34dc54dcf1c61fe8b87785124df4bc0f410
diff --git a/core/java/android/net/DnsResolver.java b/core/java/android/net/DnsResolver.java
index 7a85dcb..0b1a845 100644
--- a/core/java/android/net/DnsResolver.java
+++ b/core/java/android/net/DnsResolver.java
@@ -34,6 +34,7 @@
import android.annotation.Nullable;
import android.os.CancellationSignal;
import android.os.Looper;
+import android.os.MessageQueue;
import android.system.ErrnoException;
import android.util.Log;
@@ -466,10 +467,20 @@
private void registerFDListener(@NonNull Executor executor,
@NonNull FileDescriptor queryfd, @NonNull Callback<? super byte[]> answerCallback,
@Nullable CancellationSignal cancellationSignal, @NonNull Object lock) {
- Looper.getMainLooper().getQueue().addOnFileDescriptorEventListener(
+ final MessageQueue mainThreadMessageQueue = Looper.getMainLooper().getQueue();
+ mainThreadMessageQueue.addOnFileDescriptorEventListener(
queryfd,
FD_EVENTS,
(fd, events) -> {
+ // b/134310704
+ // Unregister fd event listener before resNetworkResult is called to prevent
+ // race condition caused by fd reused.
+ // For example when querying v4 and v6, it's possible that the first query ends
+ // and the fd is closed before the second request starts, which might return
+ // the same fd for the second request. By that time, the looper must have
+ // unregistered the fd, otherwise another event listener can't be registered.
+ mainThreadMessageQueue.removeOnFileDescriptorEventListener(fd);
+
executor.execute(() -> {
DnsResponse resp = null;
ErrnoException exception = null;
@@ -490,7 +501,11 @@
}
answerCallback.onAnswer(resp.answerbuf, resp.rcode);
});
- // Unregister this fd listener
+
+ // The file descriptor has already been unregistered, so it does not really
+ // matter what is returned here. In spirit 0 (meaning "unregister this FD")
+ // is still the closest to what the looper needs to do. When returning 0,
+ // Looper knows to ignore the fd if it has already been unregistered.
return 0;
});
}