ACPR/android_system_core
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge "libsnapshot: Fix a race condition in WaitForDelete."

Browse source
This commit is contained in:
David Anderson 2021-01-13 17:22:28 +00:00 • committed by Gerrit Code Review
commit c6b6c08f95
2 changed files with 43 additions and 26 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

49
fs_mgr/libsnapshot/snapuserd_server.cpp
View file

@ -77,9 +77,8 @@ void SnapuserdServer::ShutdownThreads() {
JoinAllThreads(); JoinAllThreads();
} }
const std::string& DmUserHandler::GetMiscName() const { DmUserHandler::DmUserHandler(std::unique_ptr<Snapuserd>&& snapuserd)
return snapuserd_->GetMiscName(); : snapuserd_(std::move(snapuserd)), misc_name_(snapuserd_->GetMiscName()) {}
}
bool SnapuserdServer::Sendmsg(android::base::borrowed_fd fd, const std::string& msg) { bool SnapuserdServer::Sendmsg(android::base::borrowed_fd fd, const std::string& msg) {
ssize_t ret = TEMP_FAILURE_RETRY(send(fd.get(), msg.data(), msg.size(), 0)); ssize_t ret = TEMP_FAILURE_RETRY(send(fd.get(), msg.data(), msg.size(), 0));
@ -148,7 +147,7 @@ bool SnapuserdServer::Receivemsg(android::base::borrowed_fd fd, const std::strin
LOG(ERROR) << "Could not find handler: " << out[1]; LOG(ERROR) << "Could not find handler: " << out[1];
return Sendmsg(fd, "fail"); return Sendmsg(fd, "fail");
} }
if ((*iter)->snapuserd()->IsAttached()) { if (!(*iter)->snapuserd() || (*iter)->snapuserd()->IsAttached()) {
LOG(ERROR) << "Tried to re-attach control device: " << out[1]; LOG(ERROR) << "Tried to re-attach control device: " << out[1];
return Sendmsg(fd, "fail"); return Sendmsg(fd, "fail");
} }
@ -185,7 +184,7 @@ bool SnapuserdServer::Receivemsg(android::base::borrowed_fd fd, const std::strin
LOG(ERROR) << "Malformed delete message, " << out.size() << " parts"; LOG(ERROR) << "Malformed delete message, " << out.size() << " parts";
return Sendmsg(fd, "fail"); return Sendmsg(fd, "fail");
} }
if (!RemoveHandler(out[1], true)) { if (!RemoveAndJoinHandler(out[1])) {
return Sendmsg(fd, "fail"); return Sendmsg(fd, "fail");
} }
return Sendmsg(fd, "success"); return Sendmsg(fd, "success");
@ -203,20 +202,38 @@ bool SnapuserdServer::Receivemsg(android::base::borrowed_fd fd, const std::strin
} }
void SnapuserdServer::RunThread(std::shared_ptr<DmUserHandler> handler) { void SnapuserdServer::RunThread(std::shared_ptr<DmUserHandler> handler) {
LOG(INFO) << "Entering thread for handler: " << handler->GetMiscName(); LOG(INFO) << "Entering thread for handler: " << handler->misc_name();
while (!StopRequested()) { while (!StopRequested()) {
if (!handler->snapuserd()->Run()) { if (!handler->snapuserd()->Run()) {
LOG(INFO) << "Snapuserd: Thread terminating";
break; break;
} }
} }
LOG(INFO) << "Exiting thread for handler: " << handler->GetMiscName(); auto misc_name = handler->misc_name();
LOG(INFO) << "Handler thread about to exit: " << misc_name;
// If the main thread called RemoveHandler, the handler was already removed {
// from within the lock, and calling RemoveHandler again has no effect. std::lock_guard<std::mutex> lock(lock_);
RemoveHandler(handler->GetMiscName(), false); auto iter = FindHandler(&lock, handler->misc_name());
if (iter == dm_users_.end()) {
// RemoveAndJoinHandler() already removed us from the list, and is
// now waiting on a join(), so just return.
LOG(INFO) << "Exiting handler thread to allow for join: " << misc_name;
return;
}
LOG(INFO) << "Exiting handler thread and freeing resources: " << misc_name;
if (handler->snapuserd()->IsAttached()) {
handler->thread().detach();
}
// Important: free resources within the lock. This ensures that if
// WaitForDelete() is called, the handler is either in the list, or
// it's not and its resources are guaranteed to be freed.
handler->FreeResources();
}
} }
bool SnapuserdServer::Start(const std::string& socketname) { bool SnapuserdServer::Start(const std::string& socketname) {
@ -351,7 +368,7 @@ bool SnapuserdServer::StartHandler(const std::shared_ptr<DmUserHandler>& handler
CHECK(!handler->snapuserd()->IsAttached()); CHECK(!handler->snapuserd()->IsAttached());
if (!handler->snapuserd()->InitBackingAndControlDevice()) { if (!handler->snapuserd()->InitBackingAndControlDevice()) {
LOG(ERROR) << "Failed to initialize control device: " << handler->GetMiscName(); LOG(ERROR) << "Failed to initialize control device: " << handler->misc_name();
return false; return false;
} }
@ -364,14 +381,14 @@ auto SnapuserdServer::FindHandler(std::lock_guard<std::mutex>* proof_of_lock,
CHECK(proof_of_lock); CHECK(proof_of_lock);
for (auto iter = dm_users_.begin(); iter != dm_users_.end(); iter++) { for (auto iter = dm_users_.begin(); iter != dm_users_.end(); iter++) {
if ((*iter)->GetMiscName() == misc_name) { if ((*iter)->misc_name() == misc_name) {
return iter; return iter;
} }
} }
return dm_users_.end(); return dm_users_.end();
} }
bool SnapuserdServer::RemoveHandler(const std::string& misc_name, bool wait) { bool SnapuserdServer::RemoveAndJoinHandler(const std::string& misc_name) {
std::shared_ptr<DmUserHandler> handler; std::shared_ptr<DmUserHandler> handler;
{ {
std::lock_guard<std::mutex> lock(lock_); std::lock_guard<std::mutex> lock(lock_);
@ -386,10 +403,8 @@ bool SnapuserdServer::RemoveHandler(const std::string& misc_name, bool wait) {
} }
auto& th = handler->thread(); auto& th = handler->thread();
if (th.joinable() && wait) { if (th.joinable()) {
th.join(); th.join();
} else if (handler->snapuserd()->IsAttached()) {
th.detach();
} }
return true; return true;
} }

20
fs_mgr/libsnapshot/snapuserd_server.h
View file

@ -46,18 +46,19 @@ enum class DaemonOperations {
}; };
class DmUserHandler { class DmUserHandler {
private:
std::thread thread_;
std::unique_ptr<Snapuserd> snapuserd_;
public: public:
explicit DmUserHandler(std::unique_ptr<Snapuserd>&& snapuserd) explicit DmUserHandler(std::unique_ptr<Snapuserd>&& snapuserd);
: snapuserd_(std::move(snapuserd)) {}
void FreeResources() { snapuserd_ = nullptr; }
const std::unique_ptr<Snapuserd>& snapuserd() const { return snapuserd_; } const std::unique_ptr<Snapuserd>& snapuserd() const { return snapuserd_; }
std::thread& thread() { return thread_; } std::thread& thread() { return thread_; }
const std::string& GetMiscName() const; const std::string& misc_name() const { return misc_name_; }
private:
std::thread thread_;
std::unique_ptr<Snapuserd> snapuserd_;
std::string misc_name_;
}; };
class Stoppable { class Stoppable {
@ -71,8 +72,9 @@ class Stoppable {
bool StopRequested() { bool StopRequested() {
// checks if value in future object is available // checks if value in future object is available
if (futureObj_.wait_for(std::chrono::milliseconds(0)) == std::future_status::timeout) if (futureObj_.wait_for(std::chrono::milliseconds(0)) == std::future_status::timeout) {
return false; return false;
}
return true; return true;
} }
// Request the thread to stop by setting value in promise object // Request the thread to stop by setting value in promise object
@ -98,7 +100,7 @@ class SnapuserdServer : public Stoppable {
bool Receivemsg(android::base::borrowed_fd fd, const std::string& str); bool Receivemsg(android::base::borrowed_fd fd, const std::string& str);
void ShutdownThreads(); void ShutdownThreads();
bool RemoveHandler(const std::string& control_device, bool wait); bool RemoveAndJoinHandler(const std::string& control_device);
DaemonOperations Resolveop(std::string& input); DaemonOperations Resolveop(std::string& input);
std::string GetDaemonStatus(); std::string GetDaemonStatus();
void Parsemsg(std::string const& msg, const char delim, std::vector<std::string>& out); void Parsemsg(std::string const& msg, const char delim, std::vector<std::string>& out);