commit b08fbde43862d3e6a559218d1ad5c6f6ddcdb9b4
author Luke Huang <huangluke@google.com> Fri Mar 29 03:15:32 2019 +0000
committer Gerrit Code Review <noreply-gerritcodereview@google.com> Fri Mar 29 03:15:32 2019 +0000
tree 9f4fcab5bf5395a40e83b9342a16384a6b9b1a05
parent f004bcbbf59c7cc6fd577f5fab5f87fde7ee00d3
parent 93ebd752c00c307805bd75ad168b2415b71ab333

Merge "Fix cancellation race problem for aysnc DNS API"

core/java/android/net/DnsResolver.java

diff --git a/core/java/android/net/DnsResolver.java b/core/java/android/net/DnsResolver.java
index 5980251..f9e0af2 100644
--- a/core/java/android/net/DnsResolver.java
+++ b/core/java/android/net/DnsResolver.java
@@ -205,6 +205,7 @@
         if (cancellationSignal != null && cancellationSignal.isCanceled()) {
             return;
         }
+        final Object lock = new Object();
         final FileDescriptor queryfd;
         try {
             queryfd = resNetworkSend((network != null
@@ -214,8 +215,8 @@
             return;
         }
 
-        maybeAddCancellationSignal(cancellationSignal, queryfd);
-        registerFDListener(executor, queryfd, callback);
+        maybeAddCancellationSignal(cancellationSignal, queryfd, lock);
+        registerFDListener(executor, queryfd, callback, cancellationSignal, lock);
     }
 
     /**
@@ -242,6 +243,7 @@
         if (cancellationSignal != null && cancellationSignal.isCanceled()) {
             return;
         }
+        final Object lock = new Object();
         final FileDescriptor queryfd;
         try {
             queryfd = resNetworkQuery((network != null
@@ -251,31 +253,37 @@
             return;
         }
 
-        maybeAddCancellationSignal(cancellationSignal, queryfd);
-        registerFDListener(executor, queryfd, callback);
+        maybeAddCancellationSignal(cancellationSignal, queryfd, lock);
+        registerFDListener(executor, queryfd, callback, cancellationSignal, lock);
     }
 
     private <T> void registerFDListener(@NonNull Executor executor,
-            @NonNull FileDescriptor queryfd, @NonNull AnswerCallback<T> answerCallback) {
+            @NonNull FileDescriptor queryfd, @NonNull AnswerCallback<T> answerCallback,
+            @Nullable CancellationSignal cancellationSignal, @NonNull Object lock) {
         Looper.getMainLooper().getQueue().addOnFileDescriptorEventListener(
                 queryfd,
                 FD_EVENTS,
                 (fd, events) -> {
                     executor.execute(() -> {
-                        byte[] answerbuf = null;
-                        try {
-                            answerbuf = resNetworkResult(fd);
-                        } catch (ErrnoException e) {
-                            Log.e(TAG, "resNetworkResult:" + e.toString());
-                            answerCallback.onQueryException(e);
-                            return;
-                        }
+                        synchronized (lock) {
+                            if (cancellationSignal != null && cancellationSignal.isCanceled()) {
+                                return;
+                            }
+                            byte[] answerbuf = null;
+                            try {
+                                answerbuf = resNetworkResult(fd);
+                            } catch (ErrnoException e) {
+                                Log.e(TAG, "resNetworkResult:" + e.toString());
+                                answerCallback.onQueryException(e);
+                                return;
+                            }
 
-                        try {
-                            answerCallback.onAnswer(
-                                    answerCallback.parser.parse(answerbuf));
-                        } catch (ParseException e) {
-                            answerCallback.onParseException(e);
+                            try {
+                                answerCallback.onAnswer(
+                                        answerCallback.parser.parse(answerbuf));
+                            } catch (ParseException e) {
+                                answerCallback.onParseException(e);
+                            }
                         }
                     });
                     // Unregister this fd listener
@@ -284,14 +292,16 @@
     }
 
     private void maybeAddCancellationSignal(@Nullable CancellationSignal cancellationSignal,
-            @NonNull FileDescriptor queryfd) {
+            @NonNull FileDescriptor queryfd, @NonNull Object lock) {
         if (cancellationSignal == null) return;
-        cancellationSignal.setOnCancelListener(
-                () -> {
-                    Looper.getMainLooper().getQueue()
-                            .removeOnFileDescriptorEventListener(queryfd);
-                    resNetworkCancel(queryfd);
-            });
+        cancellationSignal.setOnCancelListener(() -> {
+            synchronized (lock)  {
+                if (!queryfd.valid()) return;
+                Looper.getMainLooper().getQueue()
+                        .removeOnFileDescriptorEventListener(queryfd);
+                resNetworkCancel(queryfd);
+            }
+        });
     }
 
     private static class DnsAddressAnswer extends DnsPacket {

core/jni/android_net_NetUtils.cpp

diff --git a/core/jni/android_net_NetUtils.cpp b/core/jni/android_net_NetUtils.cpp
index d7a981e..82acf6f 100644
--- a/core/jni/android_net_NetUtils.cpp
+++ b/core/jni/android_net_NetUtils.cpp
@@ -470,6 +470,7 @@
     std::vector<uint8_t> buf(MAXPACKETSIZE, 0);
 
     int res = resNetworkResult(fd, &rcode, buf.data(), MAXPACKETSIZE);
+    jniSetFileDescriptorOfFD(env, javaFd, -1);
     if (res < 0) {
         throwErrnoException(env, "resNetworkResult", -res);
         return nullptr;
@@ -490,6 +491,7 @@
 static void android_net_utils_resNetworkCancel(JNIEnv *env, jobject thiz, jobject javaFd) {
     int fd = jniGetFDFromFileDescriptor(env, javaFd);
     resNetworkCancel(fd);
+    jniSetFileDescriptorOfFD(env, javaFd, -1);
 }
 
 static jobject android_net_utils_getTcpRepairWindow(JNIEnv *env, jobject thiz, jobject javaFd) {