[Thread] resolve open comments for aosp/2737373
Change-Id: Id2c58d8bf72139f10036b5488a12f45dd0c7c2db
diff --git a/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java b/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java
index 3470f27..f5ebc90 100644
--- a/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java
+++ b/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java
@@ -206,6 +206,7 @@
return mOtDaemon;
}
+ // TODO(b/309792480): restarts the OT daemon service
private void onOtDaemonDied() {
Log.w(TAG, "OT daemon became dead, clean up...");
OperationReceiverWrapper.onOtDaemonDied();
@@ -462,9 +463,15 @@
private void handleDeviceRoleChanged(@DeviceRole int deviceRole) {
if (ThreadNetworkController.isAttached(deviceRole)) {
Log.d(TAG, "Attached to the Thread network");
+
+ // This is an idempotent method which can be called for multiple times when the device
+ // is already attached (e.g. going from Child to Router)
registerThreadNetwork();
} else {
Log.d(TAG, "Detached from the Thread network");
+
+ // This is an idempotent method which can be called for multiple times when the device
+ // is already detached or stopped
unregisterThreadNetwork();
}
}
@@ -521,7 +528,7 @@
public void registerStateCallback(IStateCallback callback) {
checkOnHandlerThread();
if (mStateCallbacks.containsKey(callback)) {
- return;
+ throw new IllegalStateException("Registering the same IStateCallback twice");
}
IBinder.DeathRecipient deathRecipient =
@@ -553,7 +560,8 @@
public void registerDatasetCallback(IOperationalDatasetCallback callback) {
checkOnHandlerThread();
if (mOpDatasetCallbacks.containsKey(callback)) {
- return;
+ throw new IllegalStateException(
+ "Registering the same IOperationalDatasetCallback twice");
}
IBinder.DeathRecipient deathRecipient =
@@ -628,7 +636,7 @@
mActiveDataset = newActiveDataset;
} catch (IllegalArgumentException e) {
// Is unlikely that OT will generate invalid Operational Dataset
- Log.w(TAG, "Ignoring invalid Active Operational Dataset changes", e);
+ Log.wtf(TAG, "Invalid Active Operational Dataset from OpenThread", e);
}
PendingOperationalDataset newPendingDataset;
@@ -642,7 +650,8 @@
onPendingOperationalDatasetChanged(newPendingDataset, listenerId);
mPendingDataset = newPendingDataset;
} catch (IllegalArgumentException e) {
- Log.w(TAG, "Ignoring invalid Pending Operational Dataset changes", e);
+ // Is unlikely that OT will generate invalid Operational Dataset
+ Log.wtf(TAG, "Invalid Pending Operational Dataset from OpenThread", e);
}
}
diff --git a/thread/service/java/com/android/server/thread/TunInterfaceController.java b/thread/service/java/com/android/server/thread/TunInterfaceController.java
index ac65b11..7223b2a 100644
--- a/thread/service/java/com/android/server/thread/TunInterfaceController.java
+++ b/thread/service/java/com/android/server/thread/TunInterfaceController.java
@@ -16,6 +16,7 @@
package com.android.server.thread;
+import android.annotation.Nullable;
import android.net.LinkAddress;
import android.net.util.SocketUtils;
import android.os.ParcelFileDescriptor;
@@ -34,6 +35,7 @@
/** Controller for virtual/tunnel network interfaces. */
public class TunInterfaceController {
private static final String TAG = "TunIfController";
+ private static final long INFINITE_LIFETIME = 0xffffffffL;
static final int MTU = 1280;
static {
@@ -76,6 +78,7 @@
}
/** Returns the FD of the tunnel interface. */
+ @Nullable
public ParcelFileDescriptor getTunFd() {
return mParcelTunFd;
}
@@ -98,7 +101,7 @@
if (address.getDeprecationTime() == LinkAddress.LIFETIME_PERMANENT
|| address.getDeprecationTime() == LinkAddress.LIFETIME_UNKNOWN) {
- validLifetimeSeconds = 0xffffffffL;
+ validLifetimeSeconds = INFINITE_LIFETIME;
} else {
validLifetimeSeconds =
Math.max(
@@ -108,7 +111,7 @@
if (address.getExpirationTime() == LinkAddress.LIFETIME_PERMANENT
|| address.getExpirationTime() == LinkAddress.LIFETIME_UNKNOWN) {
- preferredLifetimeSeconds = 0xffffffffL;
+ preferredLifetimeSeconds = INFINITE_LIFETIME;
} else {
preferredLifetimeSeconds =
Math.max(
diff --git a/thread/service/jni/com_android_server_thread_TunInterfaceController.cpp b/thread/service/jni/com_android_server_thread_TunInterfaceController.cpp
index ed39fab..c56bc0b 100644
--- a/thread/service/jni/com_android_server_thread_TunInterfaceController.cpp
+++ b/thread/service/jni/com_android_server_thread_TunInterfaceController.cpp
@@ -53,25 +53,25 @@
strlcpy(ifr.ifr_name, ifName.c_str(), sizeof(ifr.ifr_name));
if (ioctl(fd, TUNSETIFF, &ifr, sizeof(ifr)) != 0) {
- close(fd);
jniThrowExceptionFmt(env, "java/io/IOException", "ioctl(TUNSETIFF) failed (%s)",
strerror(errno));
+ close(fd);
return -1;
}
int inet6 = socket(AF_INET6, SOCK_DGRAM | SOCK_CLOEXEC | SOCK_NONBLOCK, IPPROTO_IP);
if (inet6 == -1) {
- close(fd);
jniThrowExceptionFmt(env, "java/io/IOException", "create inet6 socket failed (%s)",
strerror(errno));
+ close(fd);
return -1;
}
ifr.ifr_mtu = mtu;
if (ioctl(inet6, SIOCSIFMTU, &ifr) != 0) {
- close(fd);
- close(inet6);
jniThrowExceptionFmt(env, "java/io/IOException", "ioctl(SIOCSIFMTU) failed (%s)",
strerror(errno));
+ close(fd);
+ close(inet6);
return -1;
}
@@ -94,7 +94,6 @@
}
if (ioctl(inet6, SIOCSIFFLAGS, &ifr) != 0) {
- close(inet6);
jniThrowExceptionFmt(env, "java/io/IOException", "ioctl(SIOCSIFFLAGS) failed (%s)",
strerror(errno));
}