Closed Bug 971686 Opened 7 years ago Closed 7 years ago

Backups can't be properly stored if the /tmp folder is on a different filesystem

Categories

(Toolkit :: Places, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: mak, Assigned: mak)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

writeAtomic can't ensure to be atomic when moving across filesystems, so it fails moving the file and we can't store the backups.
At this point I guess we should go back to the original solution that stored the .tmp file in the dest folder, and cleanup dangling .tmp files in maintenance.
Attached patch patch v1 (obsolete) — Splinter Review
Attachment #8374824 - Flags: review?(mano)
I will merge this into the patch in bug 824433 for uplift.
Comment on attachment 8374824 [details] [diff] [review]
patch v1

David, would you mind reviewing this? I need to uplift the whole thing soonish.
Attachment #8374824 - Flags: review?(mano) → review?(dteller)
Comment on attachment 8374824 [details] [diff] [review]
patch v1

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

Looks good to me.

::: toolkit/components/places/PlacesBackups.jsm
@@ +132,5 @@
>        let backupFolderPath = yield this.getBackupFolder();
>        let iterator = new OS.File.DirectoryIterator(backupFolderPath);
>        yield iterator.forEach(function(aEntry) {
> +        // Since this is a lazy getter, we can safely remove any .tmp file
> +        // without risking to remove an active backup.

Also, since I/O calls are serialized, we are sure that OS.File.remove cannot interrupt OS.File.writeAtomic.
Attachment #8374824 - Flags: review?(dteller) → review+
Attached patch patch v1.1Splinter Review
slightly better comment
Attachment #8374824 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/c8f504ccd3c3
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.