Closed
Bug 650831
Opened 13 years ago
Closed 12 years ago
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")
Categories
(MailNews Core :: Backend, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 13.0
People
(Reporter: zlynx, Assigned: Irving)
Details
(Keywords: dataloss)
Attachments
(3 files, 1 obsolete file)
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.
Updated•13 years ago
|
Component: General → Folder and Message Lists
QA Contact: general → folders-message-lists
Whiteboard: [wontfix?]
Comment 1•13 years ago
|
||
Jonathan, have you seen this in version 5 or newer?
Whiteboard: [wontfix?] → [closeme 2011-12-28]
Reporter | ||
Comment 2•13 years ago
|
||
(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•13 years ago
|
||
(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•13 years ago
|
||
virtualFolders.dat is opened with "Generic Write, Disposition: OverwriteIf", and write is successful.
Comment 5•13 years ago
|
||
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.
Updated•13 years ago
|
Attachment #584823 -
Attachment description: Process Monitor Log : successful write to virtualFolders.dat → Process Monitor Log : write open failure due to "SHARING VIOLATION" of virtualFolders.dat
Comment 6•13 years ago
|
||
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Updated•13 years ago
|
Severity: minor → normal
Component: Folder and Message Lists → Backend
Keywords: dataloss
OS: Windows 7 → All
Product: Thunderbird → MailNews Core
QA Contact: folders-message-lists → backend
Hardware: x86_64 → All
Version: unspecified → 9
Comment 8•13 years ago
|
||
FYI. "Safe File Writing" : NS_NewSafeLocalFileOutputStream
Updated•13 years ago
|
Summary: Virtual Folders are missing after unclean shutdown of Windows → 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")
Assignee | ||
Comment 9•12 years ago
|
||
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•12 years ago
|
||
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
Attachment #593091 -
Flags: review?(mbanner) → review-
Assignee | ||
Comment 11•12 years ago
|
||
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.
Attachment #593091 -
Attachment is obsolete: true
Attachment #599852 -
Flags: review?(mbanner)
Updated•12 years ago
|
Attachment #599852 -
Flags: review?(mbanner) → review+
Updated•12 years ago
|
Keywords: checkin-needed
Comment 12•12 years ago
|
||
http://hg.mozilla.org/comm-central/rev/91884a63348a
Keywords: checkin-needed
Updated•12 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 13.0
You need to log in
before you can comment on or make changes to this bug.
Description
•