From 7fe3bad14058783e430f60967b789307a4007809 Mon Sep 17 00:00:00 2001
From: chiachangwang <chiachangwang@google.com>
Date: Tue, 1 Aug 2023 11:31:15 +0000
Subject: [PATCH] Address leftover comment on aosp/2651440

Close the fd based on the automaticOnOffState increases the
complexity. Replace it with always dup the fd in the constructor.

Bug: 290094178
Test: atest FrameworksNetTests
Test: atest android.net.cts.ConnectivityManagerTest
Change-Id: I0133588c41dd6aca9f0ee0fcc8e4b4418e152284
---
 .../AutomaticOnOffKeepaliveTracker.java       | 31 ++++++++++---------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/service/src/com/android/server/connectivity/AutomaticOnOffKeepaliveTracker.java b/service/src/com/android/server/connectivity/AutomaticOnOffKeepaliveTracker.java
index 2131597625..4f2909c5ca 100644
--- a/service/src/com/android/server/connectivity/AutomaticOnOffKeepaliveTracker.java
+++ b/service/src/com/android/server/connectivity/AutomaticOnOffKeepaliveTracker.java
@@ -227,23 +227,28 @@ public class AutomaticOnOffKeepaliveTracker {
                     throw new IllegalArgumentException("fd can't be null with automatic "
                             + "on/off keepalives");
                 }
-                try {
-                    mFd = Os.dup(ki.mFd);
-                } catch (ErrnoException e) {
-                    Log.e(TAG, "Cannot dup fd: ", e);
-                    throw new InvalidSocketException(ERROR_INVALID_SOCKET, e);
-                }
                 mAlarmListener = () -> mConnectivityServiceHandler.obtainMessage(
                         CMD_MONITOR_AUTOMATIC_KEEPALIVE, mCallback.asBinder())
                         .sendToTarget();
             } else {
                 mAutomaticOnOffState = STATE_ALWAYS_ON;
-                // A null fd is acceptable in KeepaliveInfo for backward compatibility of
-                // PacketKeepalive API, but it must never happen with automatic keepalives.
-                // TODO : remove mFd from KeepaliveInfo or from this class.
-                mFd = ki.mFd;
                 mAlarmListener = null;
             }
+
+            // A null fd is acceptable in KeepaliveInfo for backward compatibility of
+            // PacketKeepalive API, but it must never happen with automatic keepalives.
+            // TODO : remove mFd from KeepaliveInfo.
+            mFd = dupFd(ki.mFd);
+        }
+
+        private FileDescriptor dupFd(FileDescriptor fd) throws InvalidSocketException {
+            try {
+                if (fd == null) return null;
+                return Os.dup(fd);
+            } catch (ErrnoException e) {
+                Log.e(TAG, "Cannot dup fd: ", e);
+                throw new InvalidSocketException(ERROR_INVALID_SOCKET, e);
+            }
         }
 
         @VisibleForTesting
@@ -501,11 +506,7 @@ public class AutomaticOnOffKeepaliveTracker {
         final AutomaticOnOffKeepalive autoKi;
         try {
             autoKi = target.withKeepaliveInfo(res.second);
-            // Only automatic keepalives duplicate the fd.
-            if (target.mAutomaticOnOffState != STATE_ALWAYS_ON) {
-                // Close the duplicated fd.
-                target.close();
-            }
+            target.close();
         } catch (InvalidSocketException e) {
             Log.wtf(TAG, "Fail to create AutomaticOnOffKeepalive", e);
             return;
-- 
GitLab