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
This commit is contained in:
Luke Huang
2019-06-11 14:25:45 +08:00
parent b1057f505a
commit 6c8b770a98

View File

@@ -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;
});
}