unity-jar-resolver icon indicating copy to clipboard operation
unity-jar-resolver copied to clipboard
verified publisher icon googlesamples

Fix project settings file getting corrupted

Open AliAlbarrak opened this issue 2 years ago • 7 comments

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 avatar Nov 06 '23 05:11 AliAlbarrak

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

AliAlbarrak avatar Nov 14 '23 07:11 AliAlbarrak

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 avatar Dec 17 '23 04:12 AliAlbarrak

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.

AliAlbarrak avatar Dec 24 '23 05:12 AliAlbarrak

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

Nezz avatar Jan 05 '24 09:01 Nezz

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

cmcpasserby avatar Jan 05 '24 16:01 cmcpasserby

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

Tommigun1980 avatar Jan 17 '24 13:01 Tommigun1980

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

berniegp avatar Mar 09 '24 22:03 berniegp

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

AliAlbarrak avatar May 30 '24 12:05 AliAlbarrak