Merge "libsnapshot: Fix a race condition in WaitForDelete." (c6b6c08f) · Commits · e / os / android_system_core

fs_mgr/libsnapshot/snapuserd_server.cpp

+32 −17

Original line number	Diff line number	Diff line
		@@ -77,9 +77,8 @@ void SnapuserdServer::ShutdownThreads() {
		JoinAllThreads();
		}

		const std::string& DmUserHandler::GetMiscName() const {
		return snapuserd_->GetMiscName();
		}
		DmUserHandler::DmUserHandler(std::unique_ptr<Snapuserd>&& snapuserd)
		: snapuserd_(std::move(snapuserd)), misc_name_(snapuserd_->GetMiscName()) {}

		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));
		@@ -148,7 +147,7 @@ bool SnapuserdServer::Receivemsg(android::base::borrowed_fd fd, const std::strin
		LOG(ERROR) << "Could not find handler: " << out[1];
		return Sendmsg(fd, "fail");
		}
		if ((*iter)->snapuserd()->IsAttached()) {
		if (!(iter)->snapuserd() \|\| (iter)->snapuserd()->IsAttached()) {
		LOG(ERROR) << "Tried to re-attach control device: " << out[1];
		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";
		return Sendmsg(fd, "fail");
		}
		if (!RemoveHandler(out[1], true)) {
		if (!RemoveAndJoinHandler(out[1])) {
		return Sendmsg(fd, "fail");
		}
		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) {
		LOG(INFO) << "Entering thread for handler: " << handler->GetMiscName();
		LOG(INFO) << "Entering thread for handler: " << handler->misc_name();

		while (!StopRequested()) {
		if (!handler->snapuserd()->Run()) {
		LOG(INFO) << "Snapuserd: Thread terminating";
		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.
		RemoveHandler(handler->GetMiscName(), false);
		{
		std::lock_guard<std::mutex> lock(lock_);
		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) {
		@@ -351,7 +368,7 @@ bool SnapuserdServer::StartHandler(const std::shared_ptr<DmUserHandler>& handler
		CHECK(!handler->snapuserd()->IsAttached());

		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;
		}

		@@ -364,14 +381,14 @@ auto SnapuserdServer::FindHandler(std::lock_guard<std::mutex>* proof_of_lock,
		CHECK(proof_of_lock);

		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 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::lock_guard<std::mutex> lock(lock_);
		@@ -386,10 +403,8 @@ bool SnapuserdServer::RemoveHandler(const std::string& misc_name, bool wait) {
		}

		auto& th = handler->thread();
		if (th.joinable() && wait) {
		if (th.joinable()) {
		th.join();
		} else if (handler->snapuserd()->IsAttached()) {
		th.detach();
		}
		return true;
		}

fs_mgr/libsnapshot/snapuserd_server.h

+11 −9

Original line number	Diff line number	Diff line
		@@ -46,18 +46,19 @@ enum class DaemonOperations {
		};

		class DmUserHandler {
		private:
		std::thread thread_;
		std::unique_ptr<Snapuserd> snapuserd_;

		public:
		explicit DmUserHandler(std::unique_ptr<Snapuserd>&& snapuserd)
		: snapuserd_(std::move(snapuserd)) {}
		explicit DmUserHandler(std::unique_ptr<Snapuserd>&& snapuserd);

		void FreeResources() { snapuserd_ = nullptr; }
		const std::unique_ptr<Snapuserd>& snapuserd() const { return snapuserd_; }
		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 {
		@@ -71,8 +72,9 @@ class Stoppable {

		bool StopRequested() {
		// 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 true;
		}
		// 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);

		void ShutdownThreads();
		bool RemoveHandler(const std::string& control_device, bool wait);
		bool RemoveAndJoinHandler(const std::string& control_device);
		DaemonOperations Resolveop(std::string& input);
		std::string GetDaemonStatus();
		void Parsemsg(std::string const& msg, const char delim, std::vector<std::string>& out);