Last Comment Bug 650831 - Virtual Folders are missing after unclean shutdown of Windows (Null virtualFolders.dat is generate if system crash/power failure occurs between write open and data write, and update is lost if write open fails due to "SHARING VIOLATION")
: Virtual Folders are missing after unclean shutdown of Windows (Null virtualFo...
Status: RESOLVED FIXED
: dataloss
Product: MailNews Core
Classification: Components
Component: Backend (show other bugs)
: 9
: All All
: -- critical (vote)
: Thunderbird 13.0
Assigned To: :Irving Reid (No longer working on Firefox)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-04-18 09:53 PDT by Jonathan Briggs
Modified: 2012-02-26 15:39 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Process Monitor Log : successful write to virtualFolders.dat (2.80 KB, text/plain)
2011-12-29 12:58 PST, WADA
no flags Details
Process Monitor Log : write open failure due to "SHARING VIOLATION" of virtualFolders.dat (1.36 KB, text/plain)
2011-12-29 13:02 PST, WADA
no flags Details
Use the safe file output stream to save virtualFolders.dat (3.70 KB, patch)
2012-01-31 07:30 PST, :Irving Reid (No longer working on Firefox)
standard8: review-
Details | Diff | Review
Second pass at using safe-output-stream for virtualFolders.dat (3.48 KB, patch)
2012-02-22 19:54 PST, :Irving Reid (No longer working on Firefox)
standard8: review+
Details | Diff | Review

Description Jonathan Briggs 2011-04-18 09:53:14 PDT
User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:5.0a2) Gecko/20110417 Firefox/5.0a2
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.15) Gecko/20110303 Lightning/1.0b2 Thunderbird/3.1.9

I was shutting down Windows and it got stuck during shutdown. I do not know why. Forced shutdown failed to work also. After several minutes I used the Reset button.

After booting again, Thunderbird was missing all Virtual Folders. I shut Thunderbird down and examined the virtualFolders.dat file in the profile. It was essentially empty. I restored virtualFolders.dat from my last backup and everything now seems correct.

I suspect Thunderbird had the file open and was writing to it during the shutdown process.

Maybe virtualFolders.dat needs a backup file with a tilde extension? Maybe it needs a checksum? Maybe it needs to be made into a SQLite table?

Reproducible: Didn't try

Steps to Reproduce:
1. Make a bunch of virtual folders in Thunderbird.
2. Get Windows stuck during shutdown somehow?
3. While Thunderbird is writing the virtual folders file, reset the computer.
Comment 1 Wayne Mery (:wsmwk, NI for questions) 2011-12-07 08:50:49 PST
Jonathan, have you seen this in version 5 or newer?
Comment 2 Jonathan Briggs 2011-12-07 09:05:37 PST
(In reply to Wayne Mery (:wsmwk) from comment #1)
> Jonathan, have you seen this in version 5 or newer?

To answer the question: No.

Wayne, this isn't the kind of thing that happens often. My Windows system freezes up maybe once every three months or so.

This bug report is more of an idea for some defensive coding that would preserve data even if unusual events happen. It isn't exactly the kind of thing you can reproduce and test.
Comment 3 WADA 2011-12-10 01:46:25 PST
(In reply to Jonathan Briggs from comment #0)
> I suspect Thunderbird had the file open and was writing to it during the
> shutdown process.

Following is Process Monitor log for writing of virtualFolders.dat by Tb on Win.
> ...\virtualFolders.dat IRP_MJ_CREATE            Desired Access: Generic Write, Read Attributes, Disposition: OverwriteIf, Options: Synchronous IO Non-Alert, Non-Directory File, Attributes: n/a, ShareMode: Read, Write, AllocationSize: 0, OpenResult: Overwritten
> ...\virtualFolders.dat IRP_MJ_QUERY_INFORMATION Type: QueryNameInformationFile, Name: \Documents and Settings\wada\Application Data\Thunderbird\Profiles\y46lxnrg.Boyacky-2\virtualFolders.dat
> ...\virtualFolders.dat IRP_MJ_WRITE             Offset: 0, Length: 1,351
> ...\virtualFolders.dat IRP_MJ_CLEANUP           
> ...\virtualFolders.dat IRP_MJ_CREATE            Desired Access: Generic Read, Disposition: Open, Options: Synchronous IO Non-Alert, Complete If Oplocked, Attributes: n/a, ShareMode: Read, AllocationSize: n/a, OpenResult: Opened
> ...\virtualFolders.dat IRP_MJ_READ              Offset: 0, Length: 64
> ...\virtualFolders.dat IRP_MJ_CLEANUP           
> ...\virtualFolders.dat IRP_MJ_CLOSE             
> ...\virtualFolders.dat IRP_MJ_CLOSE             
"Disposition: OverwriteIf" is seen in "IRP_MJ_CREATE Desired Access: Generic Write" log for write-open. This indicates "replace mode write".
So, if system crash occurs just after write-open, while writing(held in OS's I/O buffer), null virtualFolders.dat is created by OS, or virtualFolders.dat is lost if OS didn't finish physical directory update completely when system crash.
If some wrote data by Tb before close of file is physically written to HDD and extent information is physically written to HDD by OS, partial virtualFolders.dat
can occur.
This can be called OS's spec.

To reduce problem like above, file loss/corruption due to crash while writing file data, Mozilla has next file I/O logic like next, which developers calls "safe file writing".
 1. write data for new version of fileX.abc to fileX-1.abc
 2. delete old fileX.abc
 3. rename fileX-1.abc to fileX.abc
This logic is used for prefs.js writing.

In this case, even developers call "safe writing", file can be lost if system crash or power failure occurs after step 2 and while OS is processing request of step 3, rename. "Rename of a file" is directory update process by OS, and it consists of many internal steps and many phyisical IOs to HDD, so it is relatively long process, but power failure or system crash can happen any timing.
This also can be called OS's spec.
See bug 569683 and dependency tree for bug 193638 for phenomenon of loss or corruption of prefs.js, and see bug 415910(and bugs in depndency tree) for phenomenon after loss of prefs.js, and see bug 639485 for backups of prefs.js, please.

Your report and request is virtualFolders.dat version of these bugs for prefs.js. 

By thw way, you looks for me very lucky when you experienced crash or power down, because you lost virtualFolders.dat instead of far important prefs.js :-)
Comment 4 WADA 2011-12-29 12:58:48 PST
Created attachment 584821 [details]
Process Monitor Log : successful write to virtualFolders.dat

virtualFolders.dat is opened with "Generic Write, Disposition: OverwriteIf", and write is successful.
Comment 5 WADA 2011-12-29 13:02:08 PST
Created attachment 584823 [details]
Process Monitor Log : write open failure due to "SHARING VIOLATION" of virtualFolders.dat

virtualFolders.dat is opened with "Generic Write, Disposition: OverwriteIf", and the write open fails with "SHARING VIOLATION", because virtualFolders.dat is opened by text editor(Sakura Editor) with "write exclusive" mode.
Comment 6 WADA 2011-12-29 14:15:29 PST
If system crash/power failure etc. occurs between next two requests(between "write open" and "data write"),
> IRP_MJ_CREATE Desired Access: Generic Write, Read Attributes, Disposition: OverwriteIf, (snip) ShareMode: Read, Write, AllocationSize: 0, OpenResult: Overwritten
> IRP_MJ_WRITE Offset: 0, Length: 1,351
null virtualFolders.dat is created.
Confirming.
Comment 7 Wayne Mery (:wsmwk, NI for questions) 2011-12-29 14:42:20 PST
nice work, all!
Comment 8 WADA 2011-12-29 19:40:18 PST
FYI. "Safe File Writing" : NS_NewSafeLocalFileOutputStream
Comment 9 :Irving Reid (No longer working on Firefox) 2012-01-31 07:30:13 PST
Created attachment 593091 [details] [diff] [review]
Use the safe file output stream to save virtualFolders.dat

Moved the file open outside the iteration function after IRC discussion with David Bienvenu.

Please double-check my use of auto pointers, I'm not completely comfortable with the Mozilla idioms yet and I'd like reassurance that I haven't created a leak.
Comment 10 Mark Banner (:standard8) 2012-02-21 04:20:30 PST
Comment on attachment 593091 [details] [diff] [review]
Use the safe file output stream to save virtualFolders.dat

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

I checked for memory leaks, but there's not any issues here. Its pretty hard to go wrong with these simple uses of nsCOMPtr (when just calling functions and getting results).

::: mailnews/base/src/nsMsgAccountManager.cpp
@@ +3160,5 @@
>  {
>    if (!m_virtualFoldersLoaded)
>      return NS_OK;
> +
> +  nsCOMPtr<nsIOutputStream> outStreamSink;

Generally we prefer to declare the variables just before they are used.

@@ +3168,5 @@
> +  GetVirtualFoldersFile(file);
> +
> +  // Open a buffered, safe output stream
> +  nsresult rv = NS_NewSafeLocalFileOutputStream(getter_AddRefs(outStreamSink),
> +                                            file,

nit: params should align with the first letter after the opening bracket, or just be 2-space indented (I'd probably go the latter, and put the second, third & fourth params all on the second line).

@@ +3172,5 @@
> +                                            file,
> +                                            PR_CREATE_FILE | PR_WRONLY | PR_TRUNCATE,
> +                                            0664);
> +  if (NS_FAILED(rv)) {
> +    NS_WARNING("failed to open virtual folders save file");

Just use NS_ENSURE_SUCCESS(rv, rv) here. That gives us the error code as well as where it failed.

@@ +3176,5 @@
> +    NS_WARNING("failed to open virtual folders save file");
> +    return rv;
> +  }
> +  rv = NS_NewBufferedOutputStream(getter_AddRefs(outStream), outStreamSink, 4096);
> +  if (NS_FAILED(rv)) 

Ditto, use NS_ENSURE_SUCCESS, then if something goes wrong in debug mode we can maybe spot it.

@@ +3183,5 @@
> +  WriteLineToOutputStream("version=", "1", outStream);
> +  m_incomingServers.Enumerate(saveVirtualFolders, &outStream);
> +
> +  nsCOMPtr<nsISafeOutputStream> safeStream = do_QueryInterface(outStream);
> +  NS_ASSERTION(safeStream, "expected a safe output stream!");

Pass &rv as a second parameter to do_QueryInterface, and then do a NS_ENSURE_SUCCESS check.

@@ +3185,5 @@
> +
> +  nsCOMPtr<nsISafeOutputStream> safeStream = do_QueryInterface(outStream);
> +  NS_ASSERTION(safeStream, "expected a safe output stream!");
> +  if (safeStream) {
> +    rv = safeStream->Finish();

Do we need to use Close() on the stream as well? iirc the js does.

@@ +3186,5 @@
> +  nsCOMPtr<nsISafeOutputStream> safeStream = do_QueryInterface(outStream);
> +  NS_ASSERTION(safeStream, "expected a safe output stream!");
> +  if (safeStream) {
> +    rv = safeStream->Finish();
> +    if (NS_FAILED(rv)) {

Again, use NS_ENSURE_SUCCESS
Comment 11 :Irving Reid (No longer working on Firefox) 2012-02-22 19:54:10 PST
Created attachment 599852 [details] [diff] [review]
Second pass at using safe-output-stream for virtualFolders.dat

Address the nits in Mark's previous review.

Turns out the safe output stream finish() method calls close() on the underlying file output stream, so the close() calls in the Javascript versions (filed under other bugs) are not needed.
Comment 12 Ryan VanderMeulen [:RyanVM] 2012-02-26 12:32:58 PST
http://hg.mozilla.org/comm-central/rev/91884a63348a

Note You need to log in before you can comment on or make changes to this bug.