Work around bug in vim session storage?

[frioux]

So when vim writes a session it simply does:

open("/home/frew/.vvar/sessions/notes", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 5
write(5, "let SessionLoad = 1\nif &cp | set"..., 1215) = 1215

If there is an OS crash during the write, you can end up with broken sessions (binary sessions are clearly wrong):

$ file * 
apache-logformat-compiler.vim: ASCII text
awesome.vim:                   ASCII text
blog.vim:                      data
dotfiles.vim:                  ASCII text
feedpipe.py:                   ASCII text
keyboard.vim:                  ASCII text
lizard-brain:                  ASCII text
notes:                         ASCII text
PAWS:                          ASCII text
Plack.vim:                     ASCII text
querylog.vim:                  ASCII text
salt.vim:                      ASCII text
zr:                            data

If Obsession (or preferably vim) were to write the session to $session.tmp and then rename it to $session, the updates would be atomic. Any chance this could be a part of Obsession?

[tpope]

I would be okay with it. We should probably use a filename based on the original, e.g. g:this_obsession . &backupext.

[frioux]

Yeah that sounds like a good idea.

[frioux]

Does this look right?

$ git diff
diff --git a/plugin/obsession.vim b/plugin/obsession.vim
index 2517430..3ba95d4 100644
--- a/plugin/obsession.vim
+++ b/plugin/obsession.vim
@@ -66,7 +66,8 @@ function! s:persist() abort
       call insert(body, 'let g:this_session = v:this_session', -3)
       call insert(body, 'let g:this_obsession = v:this_session', -3)
       call insert(body, 'let g:this_obsession_status = 2', -3)
-      call writefile(body, g:this_obsession)
+      call writefile(body, g:this_obsession . &backupext)
+      call rename(g:this_obsession . &backupext, g:this_obsession)
       let g:this_session = g:this_obsession
     catch
       unlet g:this_obsession

Would be happy to submit it as a proper PR (attempting to test it against my dreaded kernel panic now, btw.)

[tpope]

You need to rope mksession in too.

[frioux]

ah thanks, how's this:

$ git diff
diff --git a/plugin/obsession.vim b/plugin/obsession.vim
index 2517430..faac150 100644
--- a/plugin/obsession.vim
+++ b/plugin/obsession.vim
@@ -61,12 +61,14 @@ function! s:persist() abort
   if exists('g:this_obsession')
     try
       set sessionoptions-=blank sessionoptions-=options sessionoptions+=tabpages
-      execute 'mksession! '.fnameescape(g:this_obsession)
-      let body = readfile(g:this_obsession)
+      let tmp = g:this_obsession . &backupext
+      execute 'mksession! '.fnameescape(tmp)
+      let body = readfile(tmp)
       call insert(body, 'let g:this_session = v:this_session', -3)
       call insert(body, 'let g:this_obsession = v:this_session', -3)
       call insert(body, 'let g:this_obsession_status = 2', -3)
-      call writefile(body, g:this_obsession)
+      call writefile(body, tmp)
+      call rename(tmp, g:this_obsession)
       let g:this_session = g:this_obsession
     catch
       unlet g:this_obsession

[frioux]

Just thought I'd leave a little heads up here. My system finally crashed again, and the fix was insufficient. Basically, while the contents of the file presumably were preserved, the metadata was not. This could be resolved with some calls to fsync or something, but it seems crazy to me to call fsync from obsession. If I have other ideas I'll post them here but I don't have a lot of hope about this whole thing. Currently I'm just gonna attempt versioning in git and committing from inotify or something.