potential race condition on session state save

[bridiver]

saving the state can be triggered by a periodic timer, the user quitting the app or the app quit timeout firing. The actual saving of the state to disk uses async io so the event loop is free to handle other method calls once it starts. This means that another call to save the state could come in and start to save while the previous call is still writing/renaming.

[BrendanEich]

And indeed I have

$ ls -l session-store-*
-rw-r--r-- 1 brendaneich staff 3046408 Aug 29 15:29 session-store-1
-rw-r--r-- 1 brendaneich staff 9956 Aug 19 17:17 session-store-1-dev
-rw-r--r-- 1 brendaneich staff 151563 Aug 5 10:34 session-store-1.new
-rw-r--r-- 1 brendaneich staff 2679227 Aug 3 23:21 session-store-tmp-1470291671487
-rw-r--r-- 1 brendaneich staff 2836106 Aug 24 21:29 session-store-tmp-1472099355984

The -dev and .new are my own doing, but the tmp ones are probably due to this bug.

[BrendanEich]

Could be dataloss, marking 1.0.

[bbondy]

Some tmp files could also be if the browser process crashes during a save.

[bridiver]

yea, that's a possibility, but the window is so small that it seems unlikely to happen twice. Either way I'm pretty sure there is a race in some cases

[bbondy]

we can't really change this to sync either so I think we need to set a value when it's possible to save again and if it's not set then don't save.

[bridiver]

well, I was thinking of an async task queue to serialize the calls to SessionStore.saveAppState. If we skip saves we could potentially lose data if the first save is a periodic save and the second is an on-quit save

[bridiver]

Testing steps:
Periodic saves

1. Open Brave
2. Change settings or close/open windows/tabs
3. Wait at least 5 minutes
4. Check brave/session-store-1 to verify that the changes have been saved
   Save on quit
5. Open Brave
6. Change settings or close/open windows/tabs
7. Restart Brave
8. Changes should still be applied