MemstickScreen: Cleanly cancel instead of blocking the back button if space calculation is underway

This commit is contained in:
Henrik Rydgård
2025-05-28 15:27:00 +02:00
parent a0ba0d2f7d
commit 6b987fa4ac
7 changed files with 95 additions and 31 deletions
+2 -2
View File
@@ -22,6 +22,7 @@
#include "Common/File/Path.h"
#include "Common/File/AndroidStorage.h"
#include "Common/Data/Encoding/Utf8.h"
#include "Common/TimeUtil.h"
#if PPSSPP_PLATFORM(UWP)
#include "UWP/UWPHelpers/StorageManager.h"
@@ -33,8 +34,7 @@ bool free_disk_space(const Path &path, int64_t &space) {
#if PPSSPP_PLATFORM(UWP)
if (GetDriveFreeSpace(path, space)) {
return true;
}
else
} else
#endif
if (GetDiskFreeSpaceExW(path.ToWString().c_str(), &free, nullptr, nullptr)) {
space = free.QuadPart;
+8 -3
View File
@@ -251,7 +251,7 @@ bool Path::StartsWith(const Path &other) const {
return startsWith(path_, other.path_);
}
bool Path::StartsWithGlobal(const Path &other) const {
bool Path::StartsWithGlobalAndNotEqual(const Path &other) const {
if (other.empty()) {
return true;
}
@@ -262,9 +262,14 @@ bool Path::StartsWithGlobal(const Path &other) const {
if (type_ == PathType::CONTENT_URI) {
AndroidContentURI a(path_);
AndroidContentURI b(other.path_);
return startsWith(a.GetLastPart(), b.GetLastPart());
std::string aLast = a.GetLastPart();
std::string bLast = b.GetLastPart();
if (aLast == bLast) {
return false;
}
return startsWith(aLast, bLast);
}
return StartsWith(other);
return *this != other && StartsWith(other);
}
const std::string &Path::ToString() const {
+2 -2
View File
@@ -126,8 +126,8 @@ public:
// WARNING: This can return wrong results if two ContentURI have different root paths.
bool StartsWith(const Path &other) const;
// This handles ContentURI with different paths "correctly".
bool StartsWithGlobal(const Path &other) const;
// This handles ContentURI with different paths "correctly". Very specific use case.
bool StartsWithGlobalAndNotEqual(const Path &other) const;
bool operator <(const Path &other) const {
return path_ < other.path_;
+35 -3
View File
@@ -44,13 +44,28 @@ public:
void Run() override {
T value = fun_();
tx_->Send(value);
if (!cancelled_) {
tx_->Send(value);
} else {
INFO_LOG(Log::System, "PromiseTask ended after cancellation");
}
}
bool Cancellable() const override {
return true;
}
void Cancel() override {
INFO_LOG(Log::System, "PromiseTask cancelled");
cancelled_ = true;
}
std::function<T ()> fun_;
Mailbox<T> *tx_;
const TaskType type_;
const TaskPriority priority_;
std::atomic<bool> cancelled_{};
};
// Represents pending or actual data.
@@ -70,6 +85,7 @@ public:
PromiseTask<T> *task = new PromiseTask<T>(fun, mailbox, taskType, taskPriority);
threadman->EnqueueTask(task);
promise->task_ = task;
return promise;
}
@@ -89,8 +105,8 @@ public:
// Allow an empty promise to spawn, too, in case we want to delay it.
void SpawnEmpty(ThreadManager *threadman, std::function<T()> fun, TaskType taskType, TaskPriority taskPriority = TaskPriority::NORMAL) {
PromiseTask<T> *task = new PromiseTask<T>(fun, rx_, taskType, taskPriority);
threadman->EnqueueTask(task);
task_ = new PromiseTask<T>(fun, rx_, taskType, taskPriority);
threadman->EnqueueTask(task_);
}
~Promise() {
@@ -110,10 +126,12 @@ public:
if (ready_) {
return data_;
} else {
_dbg_assert_(rx_);
if (rx_->Poll(&data_)) {
rx_->Release();
rx_ = nullptr;
ready_ = true;
task_ = nullptr;
return data_;
} else {
return nullptr;
@@ -128,10 +146,12 @@ public:
if (ready_) {
return data_;
} else {
_dbg_assert_(rx_);
data_ = rx_->Wait();
rx_->Release();
rx_ = nullptr;
ready_ = true;
task_ = nullptr;
return data_;
}
}
@@ -141,6 +161,17 @@ public:
rx_->Send(data);
}
void Cancel() {
std::lock_guard<std::mutex> guard(readyMutex_);
if (!ready_) {
_dbg_assert_(task_);
ready_ = true;
task_->Cancel();
rx_->Release();
rx_ = nullptr;
}
}
private:
Promise() {}
@@ -150,4 +181,5 @@ private:
std::mutex readyMutex_;
Mailbox<T> *rx_ = nullptr;
uint32_t sentinel_ = 0xffc0ffee;
PromiseTask<T> *task_ = nullptr;
};
+1 -1
View File
@@ -26,7 +26,7 @@ public:
virtual TaskType Type() const = 0;
virtual TaskPriority Priority() const = 0;
virtual void Run() = 0;
virtual bool Cancellable() { return false; }
virtual bool Cancellable() const { return false; }
virtual void Cancel() {}
virtual uint64_t id() { return 0; }
virtual void Release() { delete this; }
+24 -16
View File
@@ -460,24 +460,36 @@ ConfirmMemstickMoveScreen::ConfirmMemstickMoveScreen(const Path &newMemstickFold
: newMemstickFolder_(newMemstickFolder), initialSetup_(initialSetup), progressReporter_() {
const Path &oldMemstickFolder = g_Config.memStickDirectory;
existingFilesInNewFolder_ = FolderSeemsToBeUsed(newMemstickFolder);
folderConflict_ = newMemstickFolder.StartsWithGlobal(oldMemstickFolder);
if (!oldMemstickFolder.empty()) {
folderConflict_ = newMemstickFolder != oldMemstickFolder && newMemstickFolder.StartsWithGlobalAndNotEqual(oldMemstickFolder);
} else {
folderConflict_ = false;
}
INFO_LOG(Log::System, "Old: '%s'", oldMemstickFolder.c_str());
INFO_LOG(Log::System, "New: '%s'", newMemstickFolder.c_str());
// TODO: If you reinstall the app and start by selecting a subfolder of the PSP folder, we don't detect and warn about that -
// we can only warn if there's a known previous folder :(
if (initialSetup_) {
moveData_ = false;
}
}
ConfirmMemstickMoveScreen::~ConfirmMemstickMoveScreen() {
// We should no longer end up blocking here since the back button is disabled until the tasks are done.
if (moveDataTask_) {
INFO_LOG(Log::System, "Move Data task still running, blocking on it");
moveDataTask_->BlockUntilReady();
delete moveDataTask_;
}
// These we just cancel / leak.
if (oldSpaceTask_) {
oldSpaceTask_->BlockUntilReady();
oldSpaceTask_->Cancel();
delete oldSpaceTask_;
}
if (newSpaceTask_) {
newSpaceTask_->BlockUntilReady();
newSpaceTask_->Cancel();
delete newSpaceTask_;
}
}
@@ -504,11 +516,14 @@ void ConfirmMemstickMoveScreen::CreateViews() {
}
leftColumn->Add(new TextView(newMemstickFolder_.ToVisualString(), ALIGN_LEFT, false));
newFreeSpaceView_ = leftColumn->Add(new TextView(ms->T("Free space"), ALIGN_LEFT, false));
// TODO: Add spinner
newFreeSpaceView_ = leftColumn->Add(new TextView(ApplySafeSubstitutions("%1: ...", ms->T("Free space")), ALIGN_LEFT, false));
newSpaceTask_ = Promise<SpaceResult *>::Spawn(&g_threadManager, [&]() -> SpaceResult * {
Path newMemstickFolder = newMemstickFolder_;
newSpaceTask_ = Promise<SpaceResult *>::Spawn(&g_threadManager, [newMemstickFolder]() -> SpaceResult * {
int64_t freeSpaceNew;
free_disk_space(newMemstickFolder_, freeSpaceNew);
INFO_LOG(Log::System, "Computing free space in %s", newMemstickFolder.c_str());
free_disk_space(newMemstickFolder, freeSpaceNew);
return new SpaceResult{ freeSpaceNew };
}, TaskType::IO_BLOCKING, TaskPriority::HIGH);
@@ -528,15 +543,16 @@ void ConfirmMemstickMoveScreen::CreateViews() {
}
if (!oldMemstickFolder.empty()) {
oldSpaceTask_ = Promise<SpaceResult *>::Spawn(&g_threadManager, [&]() -> SpaceResult * {
oldSpaceTask_ = Promise<SpaceResult *>::Spawn(&g_threadManager, [oldMemstickFolder]() -> SpaceResult * {
int64_t freeSpaceOld;
INFO_LOG(Log::System, "Computing free space in %s", oldMemstickFolder.c_str());
free_disk_space(oldMemstickFolder, freeSpaceOld);
return new SpaceResult{ freeSpaceOld };
}, TaskType::IO_BLOCKING, TaskPriority::HIGH);
rightColumn->Add(new TextView(std::string(ms->T("Current")) + ":", ALIGN_LEFT, false));
rightColumn->Add(new TextView(oldMemstickFolder.ToVisualString(), ALIGN_LEFT, false));
oldFreeSpaceView_ = rightColumn->Add(new TextView(ms->T("Free space"), ALIGN_LEFT, false));
oldFreeSpaceView_ = rightColumn->Add(new TextView(ApplySafeSubstitutions("%1: ...", ms->T("Free space")), ALIGN_LEFT, false));
}
if (moveDataTask_) {
@@ -556,14 +572,6 @@ void ConfirmMemstickMoveScreen::CreateViews() {
if (moveDataTask_ && !moveDataTask_->Poll()) {
return UI::EVENT_DONE;
}
if (newSpaceTask_ && !newSpaceTask_->Poll()) {
// TODO: we should detach/cancel the task somehow instead..
return UI::EVENT_DONE;
}
if (oldSpaceTask_ && !oldSpaceTask_->Poll()) {
// TODO: we should detach/cancel the task somehow instead..
return UI::EVENT_DONE;
}
return UIScreen::OnBack(params);
});
}
@@ -2,6 +2,7 @@ package org.ppsspp.ppsspp;
import android.annotation.TargetApi;
import android.app.AlertDialog;
import android.content.Context;
import android.content.Intent;
import android.net.Uri;
import android.os.Build;
@@ -9,6 +10,8 @@ import android.os.Bundle;
import android.os.Environment;
import android.os.Looper;
import android.os.ParcelFileDescriptor;
import android.os.StatFs;
import android.os.storage.StorageVolume;
import android.provider.MediaStore;
import android.util.Log;
import android.system.StructStatVfs;
@@ -20,6 +23,7 @@ import android.provider.DocumentsContract;
import androidx.documentfile.provider.DocumentFile;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
import java.io.File;
@@ -469,14 +473,13 @@ public class PpssppActivity extends NativeActivity {
// The example in Android documentation uses this.getFilesDir for path.
// There's also a way to beg the OS for more space, which might clear caches, but
// let's just not bother with that for now.
// NOTE: This is really super slow!
@TargetApi(Build.VERSION_CODES.M)
public long contentUriGetFreeStorageSpace(String fileName) {
public long contentUriGetFreeStorageSpaceSlow(Uri uri) {
try {
Uri uri = Uri.parse(fileName);
ParcelFileDescriptor pfd = getContentResolver().openFileDescriptor(uri, "r");
if (pfd == null) {
Log.w(TAG, "Failed to get free storage space from URI: " + fileName);
Log.w(TAG, "Failed to get free storage space from URI: " + uri.toString());
return -1;
}
StructStatVfs stats = Os.fstatvfs(pfd.getFileDescriptor());
@@ -490,6 +493,22 @@ public class PpssppActivity extends NativeActivity {
return -1;
}
}
public long contentUriGetFreeStorageSpace(String str) {
Uri uri = Uri.parse(str);
if (uri == null) {
Log.e(TAG, "Failed to parse uri " + str);
return -1;
}
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
return contentUriGetFreeStorageSpaceSlow(uri);
}
// Too early Android version
return -1;
}
@TargetApi(Build.VERSION_CODES.O)
public long filePathGetFreeStorageSpace(String filePath) {
try {