diff options
| author | Luke Huang <huangluke@google.com> | 2019-03-26 15:50:10 +0800 |
|---|---|---|
| committer | Luke Huang <huangluke@google.com> | 2019-03-28 19:46:56 +0800 |
| commit | 5386f49418a62ce8564139071707c3b01fbbe3ac (patch) | |
| tree | 7a021ce73c8479f5953786ffd7304f43c6b07246 /core/java | |
| parent | eea398a690496127b3626917bd800a5d57ae911c (diff) | |
Fix cancellation race problem for aysnc DNS API
This problem might cause double-close fd and result in app crash
or unexpected behaviour
Bug: 129317069
Test: atest DnsResolverTest
manual test with delaying response callback/cancel
Change-Id: I223234f527edafc51d34fa6be390419c05def8d8
Diffstat (limited to 'core/java')
| -rw-r--r-- | core/java/android/net/DnsResolver.java | 62 |
1 files changed, 36 insertions, 26 deletions
diff --git a/core/java/android/net/DnsResolver.java b/core/java/android/net/DnsResolver.java index 59802514c7a3..f9e0af227ac3 100644 --- a/core/java/android/net/DnsResolver.java +++ b/core/java/android/net/DnsResolver.java @@ -205,6 +205,7 @@ public final class DnsResolver { if (cancellationSignal != null && cancellationSignal.isCanceled()) { return; } + final Object lock = new Object(); final FileDescriptor queryfd; try { queryfd = resNetworkSend((network != null @@ -214,8 +215,8 @@ public final class DnsResolver { return; } - maybeAddCancellationSignal(cancellationSignal, queryfd); - registerFDListener(executor, queryfd, callback); + maybeAddCancellationSignal(cancellationSignal, queryfd, lock); + registerFDListener(executor, queryfd, callback, cancellationSignal, lock); } /** @@ -242,6 +243,7 @@ public final class DnsResolver { if (cancellationSignal != null && cancellationSignal.isCanceled()) { return; } + final Object lock = new Object(); final FileDescriptor queryfd; try { queryfd = resNetworkQuery((network != null @@ -251,31 +253,37 @@ public final class DnsResolver { 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; - } - - try { - answerCallback.onAnswer( - answerCallback.parser.parse(answerbuf)); - } catch (ParseException e) { - answerCallback.onParseException(e); + 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); + } } }); // Unregister this fd listener @@ -284,14 +292,16 @@ public final class DnsResolver { } 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 { |
