889908 - OS.File.writeAtomic doesn't handle file open error for some writes

Status: RESOLVED FIXED

(Toolkit Graveyard :: OS.File, defect)

Description

[:Irving Reid (No longer working on Firefox)]

Attachment: Null check the destination file before closing

After the recent sync-write-removal patch, OS.File.writeAtomic no longer reports errors correctly for some file opens.

https://hg.mozilla.org/mozilla-central/file/2cae857c17cb/toolkit/components/osfile/osfile_shared_front.jsm#l362

   359   if (!options.flush) {
   360     // Just write, without any renaming trick
   361     let dest;
   362     try {
   363       dest = OS.File.open(path, {write: true, truncate: true});
   364       return dest.write(buffer, options);
   365     } finally {
   366       dest.close();
   367     }
   368   }

If OS.File.open() fails, the finally() block throws a JS error because dest is undefined.

Comment 1

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 770867 [details] [diff] [review]
Null check the destination file before closing

Review of attachment 770867 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/osfile/osfile_shared_front.jsm
@@ +368,2 @@
>      }
>    }

Actually, we could just move the |open| out of the try.

Comment 2

[:Irving Reid (No longer working on Firefox)]

Attachment: Move Open() outside the try/finally

Or we could do it this way...

Comment 3

[:Irving Reid (No longer working on Firefox)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a828fef4d274

Comment 4

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/a828fef4d274