Closed Bug 1005475 Opened 6 years ago Closed 6 years ago

Remove lock acquire from CacheEntry::IsFileDoomed()

Categories

(Core :: Networking: Cache, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: mayhemer, Assigned: mayhemer)

References

Details

Attachments

(1 file, 2 obsolete files)

Michal, you didn't address my review request on this at https://bugzilla.mozilla.org/show_bug.cgi?id=913808#c26.
Attached patch v1 (obsolete) — Splinter Review
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
Attachment #8416904 - Flags: review?(michal.novotny)
Attachment #8416904 - Flags: review?(michal.novotny) → review+
Regarding CacheFile::IsDoomed() - mHandle to my knowledge will never go away, right?  CacheFileHandle::IsDoomed() reads an aligned bool, so it's thread-safe.  I think that CacheFile::IsDoomed() doesn't need to lock as well.
Flags: needinfo?(michal.novotny)
Blocks: 993649
Attached patch v2 (more aggressive) (obsolete) — Splinter Review
Based on comment 2.
Attachment #8416913 - Flags: review?(michal.novotny)
https://tbpl.mozilla.org/?tree=Try&rev=9dd5d47cb23d (try -a)
https://tbpl.mozilla.org/?tree=Try&rev=6f3980478258 (talos, compare to reference 5c379a4d7577)
Flags: needinfo?(michal.novotny)
Comment on attachment 8416904 [details] [diff] [review]
v1

In case of a direct load (truncate or memory-only) mFile can be for a short time null after mFileStatus is set to NS_OK.  Those assignments must happen in different order.
Attachment #8416904 - Flags: review+
Attachment #8416913 - Flags: review?(michal.novotny)
Attached patch v3Splinter Review
https://tbpl.mozilla.org/?tree=Try&rev=d3dcea2411ef (try -a)
Attachment #8416922 - Flags: review?(michal.novotny)
Attachment #8416904 - Attachment is obsolete: true
Attachment #8416913 - Attachment is obsolete: true
Duplicate of this bug: 993649
Attachment #8416922 - Flags: review?(michal.novotny) → review+
https://hg.mozilla.org/mozilla-central/rev/0f9fb2d2f4ac
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.