Revert^2 "Use std::unique_ptr instead of ScopeGuard"
25c1a3b8543dd1756308424dd65030f90bb7a99f
Test: m
Bug: 302723053
Change-Id: Id9355c10d78d0c55afb49f512b78bb0923fbc4f7
diff --git a/libs/binder/FdTrigger.cpp b/libs/binder/FdTrigger.cpp
index 37c21bb..a1fbbf3 100644
--- a/libs/binder/FdTrigger.cpp
+++ b/libs/binder/FdTrigger.cpp
@@ -21,13 +21,15 @@
#include <poll.h>
-#include <android-base/scopeguard.h>
+#include <binder/Functional.h>
#include "RpcState.h"
#include "Utils.h"
namespace android {
+using namespace android::binder::impl;
+
std::unique_ptr<FdTrigger> FdTrigger::make() {
auto ret = std::make_unique<FdTrigger>();
#ifndef BINDER_RPC_SINGLE_THREADED
@@ -75,8 +77,7 @@
"Only one thread should be polling on Fd!");
transportFd.setPollingState(true);
- auto pollingStateGuard =
- android::base::make_scope_guard([&]() { transportFd.setPollingState(false); });
+ auto pollingStateGuard = make_scope_guard([&]() { transportFd.setPollingState(false); });
int ret = TEMP_FAILURE_RETRY(poll(pfd, countof(pfd), -1));
if (ret < 0) {
diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp
index 94851c6..94f3631 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -33,6 +33,7 @@
#include <binder/Binder.h>
#include <binder/BpBinder.h>
+#include <binder/Functional.h>
#include <binder/IPCThreadState.h>
#include <binder/Parcel.h>
#include <binder/ProcessState.h>
@@ -40,7 +41,6 @@
#include <binder/Status.h>
#include <binder/TextOutput.h>
-#include <android-base/scopeguard.h>
#ifndef BINDER_DISABLE_BLOB
#include <cutils/ashmem.h>
#endif
@@ -92,6 +92,8 @@
namespace android {
+using namespace android::binder::impl;
+
// many things compile this into prebuilts on the stack
#ifdef __LP64__
static_assert(sizeof(Parcel) == 120);
@@ -584,7 +586,7 @@
}
const size_t savedDataPos = mDataPos;
- base::ScopeGuard scopeGuard = [&]() { mDataPos = savedDataPos; };
+ auto scopeGuard = make_scope_guard([&]() { mDataPos = savedDataPos; });
rpcFields->mObjectPositions.reserve(otherRpcFields->mObjectPositions.size());
if (otherRpcFields->mFds != nullptr) {
diff --git a/libs/binder/ProcessState.cpp b/libs/binder/ProcessState.cpp
index 7af2845..0344eb0 100644
--- a/libs/binder/ProcessState.cpp
+++ b/libs/binder/ProcessState.cpp
@@ -18,9 +18,9 @@
#include <binder/ProcessState.h>
-#include <android-base/scopeguard.h>
#include <android-base/strings.h>
#include <binder/BpBinder.h>
+#include <binder/Functional.h>
#include <binder/IPCThreadState.h>
#include <binder/IServiceManager.h>
#include <binder/Stability.h>
@@ -60,6 +60,8 @@
namespace android {
+using namespace android::binder::impl;
+
class PoolThread : public Thread
{
public:
@@ -430,7 +432,7 @@
size_t ProcessState::getThreadPoolMaxTotalThreadCount() const {
pthread_mutex_lock(&mThreadCountLock);
- base::ScopeGuard detachGuard = [&]() { pthread_mutex_unlock(&mThreadCountLock); };
+ auto detachGuard = make_scope_guard([&]() { pthread_mutex_unlock(&mThreadCountLock); });
if (mThreadPoolStarted) {
LOG_ALWAYS_FATAL_IF(mKernelStartedThreads > mMaxThreads + 1,
diff --git a/libs/binder/RecordedTransaction.cpp b/libs/binder/RecordedTransaction.cpp
index 3246706..cedd3af 100644
--- a/libs/binder/RecordedTransaction.cpp
+++ b/libs/binder/RecordedTransaction.cpp
@@ -16,12 +16,13 @@
#include <android-base/file.h>
#include <android-base/logging.h>
-#include <android-base/scopeguard.h>
#include <android-base/unique_fd.h>
+#include <binder/Functional.h>
#include <binder/RecordedTransaction.h>
#include <sys/mman.h>
#include <algorithm>
+using namespace android::binder::impl;
using android::Parcel;
using android::base::borrowed_fd;
using android::base::unique_fd;
@@ -218,7 +219,7 @@
size_t memoryMappedSize = chunkPayloadSize + mmapPayloadStartOffset;
void* mappedMemory =
mmap(NULL, memoryMappedSize, PROT_READ, MAP_SHARED, fd.get(), mmapPageAlignedStart);
- auto mmap_guard = android::base::make_scope_guard(
+ auto mmap_guard = make_scope_guard(
[mappedMemory, memoryMappedSize] { munmap(mappedMemory, memoryMappedSize); });
transaction_checksum_t* payloadMap =
diff --git a/libs/binder/RpcServer.cpp b/libs/binder/RpcServer.cpp
index 8037f08..1ba20b3 100644
--- a/libs/binder/RpcServer.cpp
+++ b/libs/binder/RpcServer.cpp
@@ -25,7 +25,7 @@
#include <thread>
#include <vector>
-#include <android-base/scopeguard.h>
+#include <binder/Functional.h>
#include <binder/Parcel.h>
#include <binder/RpcServer.h>
#include <binder/RpcTransportRaw.h>
@@ -44,7 +44,7 @@
constexpr size_t kSessionIdBytes = 32;
-using base::ScopeGuard;
+using namespace android::binder::impl;
using base::unique_fd;
RpcServer::RpcServer(std::unique_ptr<RpcTransportCtx> ctx) : mCtx(std::move(ctx)) {}
@@ -454,11 +454,12 @@
LOG_ALWAYS_FATAL_IF(threadId == server->mConnectingThreads.end(),
"Must establish connection on owned thread");
thisThread = std::move(threadId->second);
- ScopeGuard detachGuard = [&]() {
+ auto detachGuardLambda = [&]() {
thisThread.detach();
_l.unlock();
server->mShutdownCv.notify_all();
};
+ auto detachGuard = make_scope_guard(std::ref(detachGuardLambda));
server->mConnectingThreads.erase(threadId);
if (status != OK || server->mShutdownTrigger->isTriggered()) {
@@ -544,7 +545,7 @@
return;
}
- detachGuard.Disable();
+ detachGuard.release();
session->preJoinThreadOwnership(std::move(thisThread));
}
diff --git a/libs/binder/RpcSession.cpp b/libs/binder/RpcSession.cpp
index 70382c0..c895b21 100644
--- a/libs/binder/RpcSession.cpp
+++ b/libs/binder/RpcSession.cpp
@@ -26,8 +26,8 @@
#include <string_view>
-#include <android-base/scopeguard.h>
#include <binder/BpBinder.h>
+#include <binder/Functional.h>
#include <binder/Parcel.h>
#include <binder/RpcServer.h>
#include <binder/RpcTransportRaw.h>
@@ -50,6 +50,7 @@
namespace android {
+using namespace android::binder::impl;
using base::unique_fd;
RpcSession::RpcSession(std::unique_ptr<RpcTransportCtx> ctx) : mCtx(std::move(ctx)) {
@@ -494,7 +495,7 @@
if (auto status = initShutdownTrigger(); status != OK) return status;
auto oldProtocolVersion = mProtocolVersion;
- auto cleanup = base::ScopeGuard([&] {
+ auto cleanup = make_scope_guard([&] {
// if any threads are started, shut them down
(void)shutdownAndWait(true);
@@ -574,7 +575,7 @@
if (status_t status = connectAndInit(mId, true /*incoming*/); status != OK) return status;
}
- cleanup.Disable();
+ cleanup.release();
return OK;
}
diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp
index 964a5c4..c090a9c 100644
--- a/libs/binder/RpcState.cpp
+++ b/libs/binder/RpcState.cpp
@@ -18,8 +18,8 @@
#include "RpcState.h"
-#include <android-base/scopeguard.h>
#include <binder/BpBinder.h>
+#include <binder/Functional.h>
#include <binder/IPCThreadState.h>
#include <binder/RpcServer.h>
@@ -38,6 +38,8 @@
namespace android {
+using namespace android::binder::impl;
+
#if RPC_FLAKE_PRONE
void rpcMaybeWaitToFlake() {
[[clang::no_destroy]] static std::random_device r;
@@ -808,11 +810,11 @@
origGuard = kernelBinderState->pushGetCallingSpGuard(&spGuard);
}
- base::ScopeGuard guardUnguard = [&]() {
+ auto guardUnguard = make_scope_guard([&]() {
if (kernelBinderState != nullptr) {
kernelBinderState->restoreGetCallingSpGuard(origGuard);
}
- };
+ });
#endif // BINDER_WITH_KERNEL_IPC
switch (command.command) {
diff --git a/libs/binder/include/binder/Functional.h b/libs/binder/include/binder/Functional.h
new file mode 100644
index 0000000..058f833
--- /dev/null
+++ b/libs/binder/include/binder/Functional.h
@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <functional>
+#include <memory>
+
+namespace android::binder::impl {
+
+template <typename F>
+constexpr void assert_small_callable() {
+ // While this buffer (std::function::__func::__buf_) is an implementation detail generally not
+ // accessible to users, it's a good bet to assume its size to be around 3 pointers.
+ constexpr size_t kFunctionBufferSize = 3 * sizeof(void*);
+
+ static_assert(sizeof(F) <= kFunctionBufferSize,
+ "Supplied callable is larger than std::function optimization buffer. "
+ "Try using std::ref, but make sure lambda lives long enough to be called.");
+}
+
+template <typename F>
+std::unique_ptr<void, std::function<void(void*)>> make_scope_guard(F&& f) {
+ assert_small_callable<decltype(std::bind(f))>();
+ return {reinterpret_cast<void*>(true), std::bind(f)};
+}
+
+} // namespace android::binder::impl
diff --git a/libs/binder/tests/binderAllocationLimits.cpp b/libs/binder/tests/binderAllocationLimits.cpp
index 6712c9c..7e0b594 100644
--- a/libs/binder/tests/binderAllocationLimits.cpp
+++ b/libs/binder/tests/binderAllocationLimits.cpp
@@ -16,6 +16,7 @@
#include <android-base/logging.h>
#include <binder/Binder.h>
+#include <binder/Functional.h>
#include <binder/IServiceManager.h>
#include <binder/Parcel.h>
#include <binder/RpcServer.h>
@@ -28,6 +29,8 @@
#include <functional>
#include <vector>
+using namespace android::binder::impl;
+
static android::String8 gEmpty(""); // make sure first allocation from optimization runs
struct DestructionAction {
@@ -172,6 +175,18 @@
a_binder->pingBinder();
}
+TEST(BinderAllocation, MakeScopeGuard) {
+ const auto m = ScopeDisallowMalloc();
+ {
+ auto guard1 = make_scope_guard([] {});
+ guard1.release();
+
+ auto guard2 = make_scope_guard([&guard1, ptr = imaginary_use] {
+ if (ptr == nullptr) guard1.release();
+ });
+ }
+}
+
TEST(BinderAllocation, InterfaceDescriptorTransaction) {
sp<IBinder> a_binder = GetRemoteBinder();
diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp
index 1ca75d1..f3969f1 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -29,11 +29,11 @@
#include <android-base/properties.h>
#include <android-base/result-gmock.h>
-#include <android-base/scopeguard.h>
#include <android-base/strings.h>
#include <android-base/unique_fd.h>
#include <binder/Binder.h>
#include <binder/BpBinder.h>
+#include <binder/Functional.h>
#include <binder/IBinder.h>
#include <binder/IPCThreadState.h>
#include <binder/IServiceManager.h>
@@ -52,6 +52,7 @@
#define ARRAY_SIZE(array) (sizeof array / sizeof array[0])
using namespace android;
+using namespace android::binder::impl;
using namespace std::string_literals;
using namespace std::chrono_literals;
using android::base::testing::HasValue;
@@ -1325,7 +1326,7 @@
ASSERT_EQ(0, ret);
// Restore the original file limits when the test finishes
- base::ScopeGuard guardUnguard([&]() { setrlimit(RLIMIT_NOFILE, &origNofile); });
+ auto guardUnguard = make_scope_guard([&]() { setrlimit(RLIMIT_NOFILE, &origNofile); });
rlimit testNofile = {1024, 1024};
ret = setrlimit(RLIMIT_NOFILE, &testNofile);