Work around the 'double files' bug.

If you click Continue fas enough, you can trigger the prepare_game_files() method to run twice, on two threads concurrently. This results in doubled up files and a duplicate files page in the navigation stack.

This is hard to fix simply or without UI ugliness, so this commit tolerates it.

It adjusts prepare_game_files() to be thread-safer (!) by committing its changes only at the end. The GIL should make this atomic, probably, so the last preparation wins instead of being combined with the other.

The navigation stack will now refuse to add a duplicate page; it still exits the old and re-enters but does not put the second entry in the history, so you'll go back further when you click the Back button.

That should hide the problem with minimal code churn and risk.
This commit is contained in:
Daniel Johnson 2024-01-13 06:04:55 -05:00
parent 0c1c8955d0
commit f56f4dd0e4
2 changed files with 13 additions and 7 deletions

View file

@ -64,7 +64,7 @@ class NavigationStack(Gtk.Stack):
In addition, this updates the navigation state so navigate_back()
and such work, they may call the presenter again.
"""
if self.current_navigated_page_presenter:
if self.current_navigated_page_presenter and self.current_navigated_page_presenter != page_presenter:
self.navigation_stack.append(self.current_navigated_page_presenter)
self._update_back_button()

View file

@ -161,11 +161,11 @@ class LutrisInstaller: # pylint: disable=too-many-instance-attributes
installer_file_id = file.id
installer_file_url = file.url
break
self.files = [file.copy() for file in self.script_files if file.id != installer_file_id]
self.extra_file_paths = []
files = [file.copy() for file in self.script_files if file.id != installer_file_id]
extra_file_paths = []
# Run variable substitution on the URLs from the script
for file in self.files:
for file in files:
file.set_url(self.interpreter._substitute(file.url))
if is_moddb_url(file.url):
file.set_url(ModDB().transform_url(file.url))
@ -178,7 +178,7 @@ class LutrisInstaller: # pylint: disable=too-many-instance-attributes
installer_files = self.service.get_patch_files(self, installer_file_id)
else:
content_files, extra_files = self.service.get_installer_files(self, installer_file_id, extras)
self.extra_file_paths = [path for f in extra_files for path in f.get_dest_files_by_id().values()]
extra_file_paths = [path for f in extra_files for path in f.get_dest_files_by_id().values()]
installer_files = content_files + extra_files
except UnavailableGameError as ex:
logger.error("Game not available: %s", ex)
@ -186,15 +186,21 @@ class LutrisInstaller: # pylint: disable=too-many-instance-attributes
if installer_files:
for installer_file in installer_files:
self.files.append(installer_file)
files.append(installer_file)
else:
# Failed to get the service game, put back a user provided file
logger.debug("Unable to get files from service. Setting %s to manual.", installer_file_id)
self.files.insert(0, InstallerFile(self.game_slug, installer_file_id, {
files.insert(0, InstallerFile(self.game_slug, installer_file_id, {
"url": installer_file_url,
"filename": ""
}))
# Commit changes only at the end; this is more robust in this method is runner
# my two threads concurrently- the GIL can probably save us. It's not desirable
# to do this, but this is the easiest workaround.
self.files = files
self.extra_file_paths = extra_file_paths
def install_extras(self):
# Copy extras to game folder; this updates the installer script, so it needs
# be called just once, before launching the installers commands.