adb: fix fd double close, Subprocess lifetime issue.
This commit fixes two somewhat related issues in shell_service.
- The fd returned by StartSubprocess is owned by a unique_fd
contained in the Subprocess object, but also gets closed by the
caller. Resolve this by duping the returned file descriptor.
- A Subprocess object can be destroyed immediately after its initial
construction in StartSubprocess if we're sufficiently unlucky.
Split up the fork/exec and "start management thread" steps, so that
we can safely do everything we need to do on the Subprocess before
handing it over to the thread that'll eventually destroy it.
Also includes squashed patches from AOSP master that allow for use of
unique_fd inside adb.
Bug: http://b/29254462
Change-Id: Id9cf0b7e7a7293bee7176919edc758597691c636
(cherry picked from commit c0e6e40cc916747a0a22c2538874348cda0ef607)
(cherry picked from commit 54c72aaccc45edf4832cfdc5053d9ae6acc9bcdf)
(cherry picked from commit 2c5d1d7cd914ec8ebf76c8a59d0889ebf5b538cd)
(cherry picked from commit 2a7b86337f7b149887588e4df532272abe3e931c)
(cherry picked from commit 13ea01db451b3993d175110a3336a58482be883d)
(cherry picked from commit 344778da411ebb47966961f3a70ca0848425194f)
diff --git a/include/android-base/errors.h b/include/android-base/errors.h
index ca621fa..04c299c 100644
--- a/include/android-base/errors.h
+++ b/include/android-base/errors.h
@@ -27,8 +27,8 @@
// special handling to get the error string. Refer to Microsoft documentation
// to determine which error code to check for each function.
-#ifndef BASE_ERRORS_H
-#define BASE_ERRORS_H
+#ifndef ANDROID_BASE_ERRORS_H
+#define ANDROID_BASE_ERRORS_H
#include <string>
@@ -43,4 +43,4 @@
} // namespace base
} // namespace android
-#endif // BASE_ERRORS_H
+#endif // ANDROID_BASE_ERRORS_H
diff --git a/include/android-base/file.h b/include/android-base/file.h
index 5342d98..aa18ea7 100644
--- a/include/android-base/file.h
+++ b/include/android-base/file.h
@@ -14,8 +14,8 @@
* limitations under the License.
*/
-#ifndef BASE_FILE_H
-#define BASE_FILE_H
+#ifndef ANDROID_BASE_FILE_H
+#define ANDROID_BASE_FILE_H
#include <sys/stat.h>
#include <string>
@@ -46,4 +46,4 @@
} // namespace base
} // namespace android
-#endif // BASE_FILE_H
+#endif // ANDROID_BASE_FILE_H
diff --git a/include/android-base/logging.h b/include/android-base/logging.h
index c82f858..d3f9d0c 100644
--- a/include/android-base/logging.h
+++ b/include/android-base/logging.h
@@ -13,8 +13,9 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-#ifndef BASE_LOGGING_H
-#define BASE_LOGGING_H
+
+#ifndef ANDROID_BASE_LOGGING_H
+#define ANDROID_BASE_LOGGING_H
// NOTE: For Windows, you must include logging.h after windows.h to allow the
// following code to suppress the evil ERROR macro:
@@ -334,4 +335,4 @@
} // namespace base
} // namespace android
-#endif // BASE_LOGGING_H
+#endif // ANDROID_BASE_LOGGING_H
diff --git a/include/android-base/macros.h b/include/android-base/macros.h
index b1ce7c6..913a9a0 100644
--- a/include/android-base/macros.h
+++ b/include/android-base/macros.h
@@ -14,8 +14,8 @@
* limitations under the License.
*/
-#ifndef UTILS_MACROS_H
-#define UTILS_MACROS_H
+#ifndef ANDROID_BASE_MACROS_H
+#define ANDROID_BASE_MACROS_H
#include <stddef.h> // for size_t
#include <unistd.h> // for TEMP_FAILURE_RETRY
@@ -185,4 +185,4 @@
} while (0)
#endif
-#endif // UTILS_MACROS_H
+#endif // ANDROID_BASE_MACROS_H
diff --git a/include/android-base/memory.h b/include/android-base/memory.h
index 882582f..3a2f8fa 100644
--- a/include/android-base/memory.h
+++ b/include/android-base/memory.h
@@ -14,8 +14,8 @@
* limitations under the License.
*/
-#ifndef BASE_MEMORY_H
-#define BASE_MEMORY_H
+#ifndef ANDROID_BASE_MEMORY_H
+#define ANDROID_BASE_MEMORY_H
namespace android {
namespace base {
@@ -44,4 +44,4 @@
} // namespace base
} // namespace android
-#endif // BASE_MEMORY_H
+#endif // ANDROID_BASE_MEMORY_H
diff --git a/include/android-base/parseint.h b/include/android-base/parseint.h
index 0543795..ed75e2d 100644
--- a/include/android-base/parseint.h
+++ b/include/android-base/parseint.h
@@ -14,8 +14,8 @@
* limitations under the License.
*/
-#ifndef BASE_PARSEINT_H
-#define BASE_PARSEINT_H
+#ifndef ANDROID_BASE_PARSEINT_H
+#define ANDROID_BASE_PARSEINT_H
#include <errno.h>
#include <stdlib.h>
@@ -70,4 +70,4 @@
} // namespace base
} // namespace android
-#endif // BASE_PARSEINT_H
+#endif // ANDROID_BASE_PARSEINT_H
diff --git a/include/android-base/parsenetaddress.h b/include/android-base/parsenetaddress.h
index 2de5ac9..b4ac025 100644
--- a/include/android-base/parsenetaddress.h
+++ b/include/android-base/parsenetaddress.h
@@ -14,8 +14,8 @@
* limitations under the License.
*/
-#ifndef BASE_PARSENETADDRESS_H
-#define BASE_PARSENETADDRESS_H
+#ifndef ANDROID_BASE_PARSENETADDRESS_H
+#define ANDROID_BASE_PARSENETADDRESS_H
#include <string>
@@ -35,4 +35,4 @@
} // namespace base
} // namespace android
-#endif // BASE_PARSENETADDRESS_H
+#endif // ANDROID_BASE_PARSENETADDRESS_H
diff --git a/include/android-base/stringprintf.h b/include/android-base/stringprintf.h
index d68af87..cf666ab 100644
--- a/include/android-base/stringprintf.h
+++ b/include/android-base/stringprintf.h
@@ -14,8 +14,8 @@
* limitations under the License.
*/
-#ifndef BASE_STRINGPRINTF_H
-#define BASE_STRINGPRINTF_H
+#ifndef ANDROID_BASE_STRINGPRINTF_H
+#define ANDROID_BASE_STRINGPRINTF_H
#include <stdarg.h>
#include <string>
@@ -53,4 +53,4 @@
} // namespace base
} // namespace android
-#endif // BASE_STRINGPRINTF_H
+#endif // ANDROID_BASE_STRINGPRINTF_H
diff --git a/include/android-base/strings.h b/include/android-base/strings.h
index 20da144..69781cd 100644
--- a/include/android-base/strings.h
+++ b/include/android-base/strings.h
@@ -14,8 +14,8 @@
* limitations under the License.
*/
-#ifndef BASE_STRINGS_H
-#define BASE_STRINGS_H
+#ifndef ANDROID_BASE_STRINGS_H
+#define ANDROID_BASE_STRINGS_H
#include <sstream>
#include <string>
@@ -65,4 +65,4 @@
} // namespace base
} // namespace android
-#endif // BASE_STRINGS_H
+#endif // ANDROID_BASE_STRINGS_H
diff --git a/include/android-base/test_utils.h b/include/android-base/test_utils.h
index 3f6872c..4ea3c8e 100644
--- a/include/android-base/test_utils.h
+++ b/include/android-base/test_utils.h
@@ -14,8 +14,8 @@
* limitations under the License.
*/
-#ifndef TEST_UTILS_H
-#define TEST_UTILS_H
+#ifndef ANDROID_BASE_TEST_UTILS_H
+#define ANDROID_BASE_TEST_UTILS_H
#include <string>
@@ -48,4 +48,4 @@
DISALLOW_COPY_AND_ASSIGN(TemporaryDir);
};
-#endif // TEST_UTILS_H
+#endif // ANDROID_BASE_TEST_UTILS_H
diff --git a/include/android-base/thread_annotations.h b/include/android-base/thread_annotations.h
index 90979df..2422102 100644
--- a/include/android-base/thread_annotations.h
+++ b/include/android-base/thread_annotations.h
@@ -14,8 +14,8 @@
* limitations under the License.
*/
-#ifndef UTILS_THREAD_ANNOTATIONS_H
-#define UTILS_THREAD_ANNOTATIONS_H
+#ifndef ANDROID_BASE_THREAD_ANNOTATIONS_H
+#define ANDROID_BASE_THREAD_ANNOTATIONS_H
#if defined(__SUPPORT_TS_ANNOTATION__) || defined(__clang__)
#define THREAD_ANNOTATION_ATTRIBUTE__(x) __attribute__((x))
@@ -80,4 +80,4 @@
#define NO_THREAD_SAFETY_ANALYSIS \
THREAD_ANNOTATION_ATTRIBUTE__(no_thread_safety_analysis)
-#endif // UTILS_THREAD_ANNOTATIONS_H
+#endif // ANDROID_BASE_THREAD_ANNOTATIONS_H
diff --git a/include/android-base/unique_fd.h b/include/android-base/unique_fd.h
index d3b27ca..869e60f 100644
--- a/include/android-base/unique_fd.h
+++ b/include/android-base/unique_fd.h
@@ -19,39 +19,54 @@
#include <unistd.h>
-#include <android-base/macros.h>
+// DO NOT INCLUDE OTHER LIBBASE HEADERS!
+// This file gets used in libbinder, and libbinder is used everywhere.
+// Including other headers from libbase frequently results in inclusion of
+// android-base/macros.h, which causes macro collisions.
-/* Container for a file descriptor that automatically closes the descriptor as
- * it goes out of scope.
- *
- * unique_fd ufd(open("/some/path", "r"));
- *
- * if (ufd.get() < 0) // invalid descriptor
- * return error;
- *
- * // Do something useful
- *
- * return 0; // descriptor is closed here
- */
+// Container for a file descriptor that automatically closes the descriptor as
+// it goes out of scope.
+//
+// unique_fd ufd(open("/some/path", "r"));
+// if (ufd.get() == -1) return error;
+//
+// // Do something useful, possibly including 'return'.
+//
+// return 0; // Descriptor is closed for you.
+//
+// unique_fd is also known as ScopedFd/ScopedFD/scoped_fd; mentioned here to help
+// you find this class if you're searching for one of those names.
namespace android {
namespace base {
-class unique_fd final {
+struct DefaultCloser {
+ static void Close(int fd) {
+ // Even if close(2) fails with EINTR, the fd will have been closed.
+ // Using TEMP_FAILURE_RETRY will either lead to EBADF or closing someone
+ // else's fd.
+ // http://lkml.indiana.edu/hypermail/linux/kernel/0509.1/0877.html
+ ::close(fd);
+ }
+};
+
+template <typename Closer>
+class unique_fd_impl final {
public:
- unique_fd() : value_(-1) {}
+ unique_fd_impl() : value_(-1) {}
- explicit unique_fd(int value) : value_(value) {}
- ~unique_fd() { clear(); }
+ explicit unique_fd_impl(int value) : value_(value) {}
+ ~unique_fd_impl() { clear(); }
- unique_fd(unique_fd&& other) : value_(other.release()) {}
- unique_fd& operator = (unique_fd&& s) {
+ unique_fd_impl(unique_fd_impl&& other) : value_(other.release()) {}
+ unique_fd_impl& operator=(unique_fd_impl&& s) {
reset(s.release());
return *this;
}
void reset(int new_value) {
- if (value_ >= 0)
- close(value_);
+ if (value_ != -1) {
+ Closer::Close(value_);
+ }
value_ = new_value;
}
@@ -60,8 +75,9 @@
}
int get() const { return value_; }
+ operator int() const { return get(); }
- int release() {
+ int release() __attribute__((warn_unused_result)) {
int ret = value_;
value_ = -1;
return ret;
@@ -70,10 +86,13 @@
private:
int value_;
- DISALLOW_COPY_AND_ASSIGN(unique_fd);
+ unique_fd_impl(const unique_fd_impl&);
+ void operator=(const unique_fd_impl&);
};
+using unique_fd = unique_fd_impl<DefaultCloser>;
+
} // namespace base
} // namespace android
-#endif // ANDROID_BASE_UNIQUE_FD_H
+#endif // ANDROID_BASE_UNIQUE_FD_H
diff --git a/include/android-base/utf8.h b/include/android-base/utf8.h
index 3b0ed0a..2d5a6f6 100755
--- a/include/android-base/utf8.h
+++ b/include/android-base/utf8.h
@@ -14,8 +14,8 @@
* limitations under the License.
*/
-#ifndef BASE_UTF8_H
-#define BASE_UTF8_H
+#ifndef ANDROID_BASE_UTF8_H
+#define ANDROID_BASE_UTF8_H
#ifdef _WIN32
#include <string>
@@ -84,4 +84,4 @@
} // namespace base
} // namespace android
-#endif // BASE_UTF8_H
+#endif // ANDROID_BASE_UTF8_H
