Fix race condition during profile switch

[jacobjmarks]

Resolves #205

This pull request resolves the following issue which is encountered when changing profiles:

Traceback (most recent call last):
  File "c:\python38\lib\site-packages\bcml\install.py", line 264, in do_and_refresh
    res = func(*args, **kwargs)
  File "c:\python38\lib\site-packages\bcml\_api.py", line 355, in set_profile
    rmtree(mod_dir)
  File "c:\python38\lib\shutil.py", line 740, in rmtree
    return _rmtree_unsafe(path, onerror)
  File "c:\python38\lib\shutil.py", line 622, in _rmtree_unsafe
    onerror(os.rmdir, path, sys.exc_info())
  File "c:\python38\lib\shutil.py", line 620, in _rmtree_unsafe
    os.rmdir(path)
OSError: [WinError 145] The directory is not empty: 'C:\\Users\\jacob\\AppData\\Local\\bcml\\mods'

After some investigation, the cause of this issue looks to be due to the execution of the get_current_profile function on another thread, during the execution of the set_profile function (and the removal of the mods folder).

This race condition has been resolved with a mutex lock.

You can find my full writeup further below.

To reproduce

1. Create a profile (A) which enables a "large" mod (such as Hyrule Rebalance)
2. Create a profile (B) which disables said mod.
3. From either profile, switch to the other (A -> B or B -> A)
4. Error encountered

Version detail

BCML 3.9.27
Python 3.8.10

Troubleshooting

See more

The problematic workflow:

1. User initiates a profile switch via the UI
2. The set_profile function starts
3. The mods directory is indexed and contents removal begins via shutil.rmdir
   • The .profile file is removed
4. The get_current_profile function is executed on another thread
   • This looks to be due to the execution of ProfileModal.refreshProfiles as part of ProfileModal.componentDidUpdate within /src/js/Profile.jsx
5. get_current_profile does not find a .profile file, and so it creates a new (default) one
6. The shutil.rmdir (still as part of set_profile) attempts to finish by removing the mods folder (which should be empty)
7. Error is encountered as the folder is not empty (a new .profile file exists)

In addition, I did not encounter this issue when there were only minor changes to implement as part of the profile switch. However, when the profile switch needed to do more work - such as toggling the Hyrule Rebalance mod - that's when these issues were observed.

Solution A - Ignorance

Adding the ignore_errors and dirs_exist_ok arguments to the rmtree and copytree calls (within the set_profile function), respectively, will resolve the issue in this case.

rmtree(mod_dir, ignore_errors=True)
copytree(params["profile"], mod_dir, dirs_exist_ok=True)

With this fix, the contents of the .profile file, during a profile switch, can be observed as follows:

{Current Profile} -> Default -> {New Profile}

This solution seemingly resolves the issue however the underlying problem (a race condition) still exists, and I believe under the right circumstances, a Default profile file would be created after a legitimate profile switch, leaving the mods folder in an invalid state.

Solution B - Mutex lock (this PR)

A more robust solution is to eliminate the race condition on the mods folder. In this case¹, specifically between get_current_profile and set_profile.

This is done with a simple mutex lock via threading.Lock - as seen in this PR - which prevents the profile check (and possible creation of a default project file) from occurring while a profile switch is in progress, and vice versa.

With this fix, the contents of the .profile file, during a profile switch, can be observed as follows:

{Current Profile} -> {New Profile}

As can be seen, no unnecessary intermediate Default profile is being created when compared to Solution A.

Footnotes

1. Ideally, other destructive operations on the mods folder would be updated to utilise this new Mutex lock. ↩

[GingerAvalanche]

Thank you for the in-depth write-up