commit 4f22786cc9c9b22d353487678058e2075a968a9d
author Tom Cherry <tomcherry@google.com> Mon Oct 08 17:33:50 2018 -0700
committer Tom Cherry <tomcherry@google.com> Fri Oct 12 18:28:59 2018 -0700
tree a310d4bcf5d6fc314df454fc8aa1c431ed8703b1
parent b3bc8427505c55a4134eac438f118a9b1acb4e2f

logd: rework logic for LogTimeEntry

LogTimeEntry's lifecycle is spread out in various locations.  It
further seems incomplete as there is logic that assumes that its
associated thread can exit while the underlying LogTimeEntry remains
valid, however it doesn't appear that that is actually a supported
situation.

This change simplifies this logic to have only one valid state for a
LogTimeEntry: it must have its thread running and be present in
LastLogTimes.  A LogTimeEntry will never be placed into LastLogTimes
unless its thread is running and its thread will remove its associated
LogTimeEntry from LastLogTimes before it has exited.

This admittedly breaks situations where a blocking socket gets issued
multiple commands with different pid filters, tail lines, etc,
however, I'm reasonably sure that these situations were already
broken.  A check is added to close the socket in this case.

Test: multiple logcat instances work, logd.reader.per's are cleaned up
Change-Id: Ibe8651e7d530c5e9a8d6ce3150cd247982887cbe

logd/LogTimes.h

diff --git a/logd/LogTimes.h b/logd/LogTimes.h
index 76d016c..f4e165f 100644
--- a/logd/LogTimes.h
+++ b/logd/LogTimes.h
@@ -22,6 +22,7 @@
 #include <time.h>
 
 #include <list>
+#include <memory>
 
 #include <log/log.h>
 #include <sysutils/SocketClient.h>
@@ -33,16 +34,12 @@
 
 class LogTimeEntry {
     static pthread_mutex_t timesLock;
-    unsigned int mRefCount;
-    bool mRelease;
-    bool mError;
-    bool threadRunning;
+    bool mRelease = false;
     bool leadingDropped;
     pthread_cond_t threadTriggeredCondition;
     pthread_t mThread;
     LogReader& mReader;
     static void* threadStart(void* me);
-    static void threadStop(void* me);
     const log_mask_t mLogMask;
     const pid_t mPid;
     unsigned int skipAhead[LOG_ID_MAX];
@@ -73,11 +70,8 @@
         pthread_mutex_unlock(&timesLock);
     }
 
-    void startReader_Locked(void);
+    bool startReader_Locked();
 
-    bool runningReader_Locked(void) const {
-        return threadRunning || mRelease || mError || mNonBlock;
-    }
     void triggerReader_Locked(void) {
         pthread_cond_signal(&threadTriggeredCondition);
     }
@@ -87,54 +81,11 @@
     }
     void cleanSkip_Locked(void);
 
-    // These called after LogTimeEntry removed from list, lock implicitly held
-    void release_nodelete_Locked(void) {
-        mRelease = true;
-        pthread_cond_signal(&threadTriggeredCondition);
-        // assumes caller code path will call decRef_Locked()
-    }
-
     void release_Locked(void) {
         mRelease = true;
         pthread_cond_signal(&threadTriggeredCondition);
-        if (mRefCount || threadRunning) {
-            return;
-        }
-        // No one else is holding a reference to this
-        delete this;
     }
 
-    // Called to mark socket in jeopardy
-    void error_Locked(void) {
-        mError = true;
-    }
-    void error(void) {
-        wrlock();
-        error_Locked();
-        unlock();
-    }
-
-    bool isError_Locked(void) const {
-        return mRelease || mError;
-    }
-
-    // Mark Used
-    //  Locking implied, grabbed when protection around loop iteration
-    void incRef_Locked(void) {
-        ++mRefCount;
-    }
-
-    bool owned_Locked(void) const {
-        return mRefCount != 0;
-    }
-
-    void decRef_Locked(void) {
-        if ((mRefCount && --mRefCount) || !mRelease || threadRunning) {
-            return;
-        }
-        // No one else is holding a reference to this
-        delete this;
-    }
     bool isWatching(log_id_t id) const {
         return mLogMask & (1 << id);
     }
@@ -146,6 +97,6 @@
     static int FilterSecondPass(const LogBufferElement* element, void* me);
 };
 
-typedef std::list<LogTimeEntry*> LastLogTimes;
+typedef std::list<std::unique_ptr<LogTimeEntry>> LastLogTimes;
 
 #endif  // _LOGD_LOG_TIMES_H__