fix bugs with download cancellation

This commit is contained in:
dankmeme01 2024-05-30 18:11:18 +02:00
parent be3af759ec
commit 2138579d14
2 changed files with 54 additions and 35 deletions

View file

@ -21,7 +21,7 @@ public:
DownloadStatus m_status;
EventListener<ServerRequest<ServerModVersion>> m_infoListener;
EventListener<web::WebTask> m_downloadListener;
Impl(
std::string const& id,
std::optional<VersionInfo> const& version,
@ -39,7 +39,7 @@ public:
if (result->isOk()) {
auto data = result->unwrap();
m_version = data.metadata.getVersion();
// Start downloads for any missing required dependencies
for (auto dep : data.metadata.getDependencies()) {
if (!dep.mod && dep.importance != ModMetadata::Dependency::Importance::Suggested) {
@ -72,7 +72,7 @@ public:
m_status = DownloadStatusCancelled();
m_infoListener.setFilter(ServerRequest<ServerModVersion>());
}
if (!ModDownloadManager::get()->checkAutoConfirm()) {
ModDownloadEvent(m_id).post();
}
@ -188,17 +188,17 @@ public:
void cancelOrphanedDependencies() {
// "This doesn't handle circular dependencies!!!!"
// Well OK and the human skull doesn't handle the 5000 newtons
// Well OK and the human skull doesn't handle the 5000 newtons
// of force from this anvil I'm about to drop on your head
for (auto& [_, d] : m_downloads) {
if (auto depFor = d.m_impl->m_dependencyFor) {
if (
!m_downloads.contains(depFor->first) ||
!(m_downloads.at(depFor->first).isActive() || m_downloads.at(depFor->first).isDone())
std::holds_alternative<DownloadStatusError>(m_downloads.at(depFor->first).getStatus())
) {
// d.cancel() will cause cancelOrphanedDependencies() to be called again
// We want that anyway because cancelling one dependency might cause
// We want that anyway because cancelling one dependency might cause
// dependencies down the chain to become orphaned
return d.cancel();
}
@ -212,8 +212,10 @@ void ModDownload::cancel() {
m_impl->m_status = DownloadStatusCancelled();
m_impl->m_infoListener.getFilter().cancel();
m_impl->m_infoListener.setFilter(ServerRequest<ServerModVersion>());
m_impl->m_downloadListener.getFilter().cancel();
m_impl->m_downloadListener.setFilter({});
// Cancel any dependencies of this mod left over (unless some other
// Cancel any dependencies of this mod left over (unless some other
// installation depends on them still)
ModDownloadManager::get()->m_impl->cancelOrphanedDependencies();
ModDownloadEvent(m_impl->m_id).post();
@ -225,9 +227,9 @@ std::optional<ModDownload> ModDownloadManager::startDownload(
std::optional<VersionInfo> const& version,
std::optional<DependencyFor> const& dependencyFor
) {
// If this mod has already been succesfully downloaded or is currently
// being downloaded, return as you can't download multiple versions of the
// same mod simultaniously, since that wouldn't make sense. I mean the new
// If this mod has already been succesfully downloaded or is currently
// being downloaded, return as you can't download multiple versions of the
// same mod simultaniously, since that wouldn't make sense. I mean the new
// version would just immediately override to the other one
if (m_impl->m_downloads.contains(id)) {
// If the download errored last time, then we can try again
@ -238,7 +240,7 @@ std::optional<ModDownload> ModDownloadManager::startDownload(
else return std::nullopt;
}
// Start a new download by constructing a ModDownload (which starts the
// Start a new download by constructing a ModDownload (which starts the
// download)
m_impl->m_downloads.emplace(id, ModDownload(id, version, dependencyFor));
return m_impl->m_downloads.at(id);
@ -279,13 +281,13 @@ bool ModDownloadManager::checkAutoConfirm() {
auto status = download.getStatus();
if (auto confirm = std::get_if<server::DownloadStatusConfirm>(&status)) {
for (auto inc : confirm->version.metadata.getIncompatibilities()) {
// If some mod has an incompatability that is installed,
// If some mod has an incompatability that is installed,
// we need to ask for confirmation
if (inc.mod) {
return false;
}
}
// If some installed mod is incompatible with this one,
// If some installed mod is incompatible with this one,
// we need to ask for confirmation
for (auto mod : Loader::get()->getAllMods()) {
for (auto inc : mod->getMetadata().getIncompatibilities()) {

View file

@ -7,31 +7,31 @@ using namespace server;
#define GEODE_GD_VERSION_STR GEODE_STR(GEODE_GD_VERSION)
template <class K, class V>
template <class K, class V>
requires std::equality_comparable<K> && std::copy_constructible<K>
class CacheMap final {
private:
// I know this looks like a goofy choice over just
// I know this looks like a goofy choice over just
// `std::unordered_map`, but hear me out:
//
// This needs preserved insertion order (so shrinking the cache
// to match size limits doesn't just have to erase random
// elements)
//
// If this used a map for values and another vector for storing
// insertion order, it would have a pretty big memory footprint
// (two copies of Query, one for order, one for map + two heap
//
// This needs preserved insertion order (so shrinking the cache
// to match size limits doesn't just have to erase random
// elements)
//
// If this used a map for values and another vector for storing
// insertion order, it would have a pretty big memory footprint
// (two copies of Query, one for order, one for map + two heap
// allocations on top of that)
//
// In addition, it would be a bad idea to have a cache of 1000s
// of items in any case (since that would likely take up a ton
// of memory, which we want to avoid since it's likely many
// crashes with the old index were due to too much memory
//
// In addition, it would be a bad idea to have a cache of 1000s
// of items in any case (since that would likely take up a ton
// of memory, which we want to avoid since it's likely many
// crashes with the old index were due to too much memory
// usage)
//
// Linear searching a vector of at most a couple dozen items is
// lightning-fast (🚀), and besides the main performance benefit
// comes from the lack of a web request - not how many extra
//
// Linear searching a vector of at most a couple dozen items is
// lightning-fast (🚀), and besides the main performance benefit
// comes from the lack of a web request - not how many extra
// milliseconds we can squeeze out of a map access
std::vector<std::pair<K, V>> m_values;
size_t m_sizeLimit = 20;
@ -121,6 +121,13 @@ public:
m_cache.add(Extract::key(args...), ServerRequest<Value>(f));
return f;
}
template <class... Args>
void remove(Args const&... args) {
std::unique_lock lock(m_mutex);
m_cache.remove(Extract::key(args...));
}
size_t size() {
std::unique_lock lock(m_mutex);
return m_cache.size();
@ -433,7 +440,7 @@ Result<ServerModMetadata> ServerModMetadata::parse(matjson::Value const& raw) {
if (res.versions.empty()) {
return Err("Mod '{}' has no (valid) versions", res.id);
}
for (auto item : root.has("tags").iterate()) {
res.tags.insert(item.template get<std::string>());
}
@ -598,7 +605,17 @@ ServerRequest<ServerModMetadata> server::getMod(std::string const& id, bool useC
ServerRequest<ServerModVersion> server::getModVersion(std::string const& id, ModVersion const& version, bool useCache) {
if (useCache) {
return getCache<getModVersion>().get(id, version);
auto& cache = getCache<getModVersion>();
auto cachedRequest = cache.get(id, version);
// if mod installation was cancelled, remove it from cache and fetch again
if (cachedRequest.isCancelled()) {
cache.remove(id, version);
return cache.get(id, version);
} else {
return cachedRequest;
}
}
auto req = web::WebRequest();
@ -721,7 +738,7 @@ ServerRequest<std::vector<ServerModUpdate>> server::checkAllUpdates(bool useCach
Loader::get()->getAllMods(),
[](auto mod) { return mod->getID(); }
);
auto req = web::WebRequest();
req.userAgent(getServerUserAgent());
req.param("platform", GEODE_PLATFORM_SHORT_IDENTIFIER);