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 Merged-In: I443bb11b15845b079ee4370a7797e692e62fa3c8 (cherry picked from commit 07de4cf82ac09f8b9f37afa9eb1b7a44b43b6fe6) Change-Id: I7041e67d8c906cbf88050e7d94245f8e15dcdbb4
This commit is contained in:
@@ -34,6 +34,7 @@ import android.annotation.NonNull;
|
||||
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 @@ public final class DnsResolver {
|
||||
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 @@ public final class DnsResolver {
|
||||
}
|
||||
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;
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user