search

googlesamples / unity-jar-resolver

Unity plugin which resolves Android & iOS dependencies and performs version management
Other
1.21k stars 336 forks source link

Fix project settings file getting corrupted #652

Closed AliAlbarrak closed 1 month ago

AliAlbarrak commented 7 months ago

Why this change?

This resolve the settings file corruption as described in #524

Even though reading and writing of the settings file is protected by a lock, it is possible (due to domain reload) that 2 separate running processes are trying to access the file at the same time so the lock in one process is not aware of the other process.

A scenario like this could happen.

  1. Code in domain A will start writing to ProjectsSettings file
  2. Code in domain B will start reading the file
  3. If domain A didn't finish writing yet, domain B will encounter unexpected EOF
  4. Domain B will assume the file is broken and reset settings
  5. Domain B will write default settings to ProjectsSettings file

What did I do?

I made the writing happen to a temporary file. When the writing is complete, copy the temp file to the target path. This avoids the file being in an "invalid state" for a short time during the writing.

Review

Feedback is greatly appreciated and please let me know which steps I need to take to make my change release-ready.

AliAlbarrak commented 7 months ago

As reported here, this fix fails sometimes so I'll draft the pull request until I find a better solution

AliAlbarrak commented 6 months ago

The settings file gets written multiple times with every domain reload/compile, even when settings didn't change. I added an extra check to skip file write if settings didn't change since last read.

AliAlbarrak commented 6 months ago

Hello @a-maurice @chkuang-g and team I'd appreciate it if you can guide me through what I need to do to get this reviewed and merged. This PR is associated with the top voted 👍 issue #524. I believe it will be great UX improvement for most users to get it fixed.

Nezz commented 5 months ago

We'd love to have this incorporated - this is a very annoying issue.

cmcpasserby commented 5 months ago

really can non one review this and get it merged in? This issue has been going on for years and makes working with this very annoying and tedious

Tommigun1980 commented 5 months ago

I tested this pull request and my findings are at https://github.com/googlesamples/unity-jar-resolver/issues/524#issuecomment-1895834683

berniegp commented 3 months ago

Hi! I still got settings corruption with your version. However I think I found the source of the problem and a fix here #678.

AliAlbarrak commented 1 month ago

I guess this is no longer needed since https://github.com/googlesamples/unity-jar-resolver/pull/678 was merged 🎉