Race condition between nsNSSSocketInfo::SetNotificationCallbacks and nsNSSSocketInfo::EnsureDocShellDependentStuffKnown

RESOLVED WORKSFORME

Status

()

defect
RESOLVED WORKSFORME
9 years ago
7 years ago

People

(Reporter: mayhemer, Assigned: briansmith)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [psm-fatal])

(Reporter)

Description

9 years ago
We count on pressumtion EnsureDocShellDependentStuffKnown is always called before SetNotificationCallbacks(null).  But in some cases, rare in release optimized builds, pretty often in debug builds running in a debugger [ lucky, isn't it ?:) ] we may get reversed order.

mCallbacks is also accessed w/o synchronization on two threads, see catched stacktraces (tracepoints in VC++):

Thread: 0x114C _threadstartex
        xul.dll!nsNSSSocketInfo::SetNotificationCallbacks() 
	xul.dll!nsSocketTransport::OnSocketDetached() 
	xul.dll!nsSocketTransportService::DetachSocket() 
	xul.dll!nsSocketTransportService::DoPollIteration() 
	xul.dll!nsSocketTransportService::OnProcessNextEvent() 
	xul.dll!nsThread::ProcessNextEvent() 
	xul.dll!NS_ProcessPendingEvents_P() 
	xul.dll!nsSocketTransportService::Run() 
	xul.dll!nsThread::ProcessNextEvent() 
	xul.dll!NS_ProcessNextEvent_P() 
	xul.dll!nsThread::ThreadFunc() 
	nspr4.dll!_PR_NativeRunThread() 
	nspr4.dll!pr_root() 
	msvcr90d.dll!_callthreadstartex() 
	msvcr90d.dll!_threadstartex() 
	kernel32.dll!761d3677() 
	[Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll]
	ntdll.dll!76ef9d42() 
	ntdll.dll!76ef9d15() 

	this = 0x09d73484 {mRefCnt={...} _mOwningThread={...} mCallbacks={...} ...}


Thread: 0x1258 _threadstartex 	xul.dll!nsNSSSocketInfo::EnsureDocShellDependentStuffKnown() 
	xul.dll!nsNSSSocketInfo::GetExternalErrorReporting() 
	xul.dll!nsNSSBadCertHandler() 
	ssl3.dll!ssl3_HandleCertificate() 
	ssl3.dll!ssl3_HandleHandshakeMessage() 
	ssl3.dll!ssl3_HandleHandshake() 
	ssl3.dll!ssl3_HandleRecord() 
	ssl3.dll!ssl3_GatherCompleteHandshake() 
	ssl3.dll!ssl_GatherRecord1stHandshake() 
	ssl3.dll!ssl_Do1stHandshake() 
	ssl3.dll!ssl_SecureSend() 
	ssl3.dll!ssl_SecureWrite() 
	ssl3.dll!ssl_Write() 
	xul.dll!nsSSLThread::Run() 
	xul.dll!nsPSMBackgroundThread::nsThreadRunner() 
	nspr4.dll!_PR_NativeRunThread() 
	nspr4.dll!pr_root() 
	msvcr90d.dll!_callthreadstartex() 
	msvcr90d.dll!_threadstartex() 
	kernel32.dll!761d3677() 
	[Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll]
	ntdll.dll!76ef9d42() 
	ntdll.dll!76ef9d15() 

	this = 0x09d73480 {mRefCnt={...} _mOwningThread={...} mCallbacks={...} ...}


I just don't get it how it is possible we first report an error to the socket and then get call to ssl_Write that bubbles to the badCertCallback.  This might be problem in the PSM background SSL thread syncing or we simply use bad nsNSSSocketInfo to call back to.

This is present at least in Fifefox 3.5.15.  So no new bug.

Updated

8 years ago
Whiteboard: [psm-fatal]

Comment 1

8 years ago
See also bug 636810
I already have patches in my local queue that remove EnsureDocShellDependentStuffKnown completely, as part of my work on removing XPCOM proxies from PSM.
Assignee: nobody → bsmith
EnsureDocShellDependentStuffKnown was removed as part of SSL thread removal / XPCOM proxy removal. We query the callbacks for the docshell info on each access now, and the main thread and the socket transport thread are blocked when doing so. So, there should be no more race condition.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Depends on: 674147
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.