Debug Thread of nsSocketTransportService::DebugMutexAutoLock not threadsafe

RESOLVED FIXED in Firefox 49

Status

()

Core
Networking
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mcmanus, Assigned: mcmanus)

Tracking

48 Branch
mozilla49
Points:
---

Firefox Tracking Flags

(firefox49 fixed)

Details

(Whiteboard: [necko-active])

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
over in 1152048 we were looking for cases where the socketthread lock was obtained re-entrantly by adding a DIAGNOSTIC_ASSERT.. and we found and fixed one in the ~42 era.

Unfortunately there are a few crashes from that DIAGNOSTIC_ASSERT (which only happens on nighty/aurora) still hanging around in current builds that don't seem related to recursive locking.

This one is from 48 https://crash-stats.mozilla.com/report/index/270eb52d-4d51-40ac-8cfd-d4bb42160526#allthreads

everything in that stack trace looks fine to me.

however, the debug thread member variable is defined as relaxed - which provides no memory ordering guarnatees.
    static Atomic<PRThread *, Relaxed> sDebugOwningThread;

based on the fact that I don't see crashes of the flavor we are looking for, and this is a red herring I am posting a patch that removes the AutoLock wrapper and just returns it to a straight autolock.
(Assignee)

Updated

2 years ago
Assignee: nobody → mcmanus
Whiteboard: [necko-active]
(Assignee)

Comment 2

2 years ago
Created attachment 8759817 [details] [diff] [review]
atomic fix to sockettransport::debugautomutex
Attachment #8759817 - Flags: review?(hurley)
Attachment #8759817 - Flags: review?(hurley) → review+

Comment 4

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f457175b1c4f
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.