exynos9610: USB: Fix potential racing by wall time change
authorWei Wang <wvw@google.com>
Fri, 11 Aug 2017 22:05:46 +0000 (15:05 -0700)
committerJan Altensen <info@stricted.net>
Sun, 1 Nov 2020 02:06:29 +0000 (03:06 +0100)
 in USBhal

pthread_cond_timedwait used wall time so it might introduce racing,
pthread_cond_timedwait_monotonic_np is Android specific but it is being
deprecated. And now Android support pthread_condattr_setclock so use it
to wait on CLOCK_MONOTONIC instead.

Bug: 64623895
Test: USB switch function works, charging/MTP/PTP
Change-Id: I136533ff90ef1be2b042ef1e0829643f2f7aa968

hidl/usb/Usb.cpp
hidl/usb/Usb.h

index 3fc188e69a412aab0afebfee8508564c1c143d7e..7b084500cde2325235f56e31e5b852fe2cde751f 100644 (file)
@@ -182,12 +182,12 @@ bool switchMode(const hidl_string &portName,
 
     if (ret != EOF) {
       struct timespec   to;
-      struct timeval    tp;
+      struct timespec   now;
 
 wait_again:
-      gettimeofday(&tp, NULL);
-      to.tv_sec = tp.tv_sec + PORT_TYPE_TIMEOUT;
-      to.tv_nsec = tp.tv_usec * 1000;;
+      clock_gettime(CLOCK_MONOTONIC, &now);
+      to.tv_sec = now.tv_sec + PORT_TYPE_TIMEOUT;
+      to.tv_nsec = now.tv_nsec;
 
       int err = pthread_cond_timedwait(&usb->mPartnerCV, &usb->mPartnerLock, &to);
       // There are no uevent signals which implies role swap timed out.
@@ -212,6 +212,29 @@ wait_again:
   return roleSwitch;
 }
 
+Usb::Usb()
+        : mLock(PTHREAD_MUTEX_INITIALIZER),
+          mRoleSwitchLock(PTHREAD_MUTEX_INITIALIZER),
+          mPartnerLock(PTHREAD_MUTEX_INITIALIZER),
+          mPartnerUp(false) {
+    pthread_condattr_t attr;
+    if (pthread_condattr_init(&attr)) {
+        ALOGE("pthread_condattr_init failed: %s", strerror(errno));
+        abort();
+    }
+    if (pthread_condattr_setclock(&attr, CLOCK_MONOTONIC)) {
+        ALOGE("pthread_condattr_setclock failed: %s", strerror(errno));
+        abort();
+    }
+    if (pthread_cond_init(&mPartnerCV, &attr))  {
+        ALOGE("pthread_cond_init failed: %s", strerror(errno));
+        abort();
+    }
+    if (pthread_condattr_destroy(&attr)) {
+        ALOGE("pthread_condattr_destroy failed: %s", strerror(errno));
+        abort();
+    }
+}
 
 
 Return<void> Usb::switchRole(const hidl_string &portName,
index dfd48ca52dad2f30d5ca6908c05856c5d2e3d6b4..78be2f9040f60fb133ffc00f7d1b9b4036222ad3 100644 (file)
@@ -40,6 +40,8 @@ using ::android::hardware::Void;
 using ::android::sp;
 
 struct Usb : public IUsb {
+    Usb();
+
     Return<void> switchRole(const hidl_string& portName, const PortRole& role) override;
     Return<void> setCallback(const sp<V1_0::IUsbCallback>& callback) override;
     Return<void> queryPortStatus() override;
@@ -47,13 +49,13 @@ struct Usb : public IUsb {
 
     sp<V1_0::IUsbCallback> mCallback_1_0;
     // Protects mCallback variable
-    pthread_mutex_t mLock = PTHREAD_MUTEX_INITIALIZER;
+    pthread_mutex_t mLock;
     // Protects roleSwitch operation
-    pthread_mutex_t mRoleSwitchLock = PTHREAD_MUTEX_INITIALIZER;
+    pthread_mutex_t mRoleSwitchLock;
     // Threads waiting for the partner to come back wait here
-    pthread_cond_t mPartnerCV = PTHREAD_COND_INITIALIZER;
+    pthread_cond_t mPartnerCV;
     // lock protecting mPartnerCV
-    pthread_mutex_t mPartnerLock = PTHREAD_MUTEX_INITIALIZER;
+    pthread_mutex_t mPartnerLock;
     // Variable to signal partner coming back online after type switch
     bool mPartnerUp;