Fix for PDT+ bug 27496 "DNS Service is not threadsafe on Windows". Added locks around WSAAsyncGetHostByName to protect against the possibility of the call completing before it returns. The lookup object could get deleted before the results were assigned to it.

This commit is contained in:
gordon%netscape.com 2000-02-27 23:06:00 +00:00
parent 90533efa40
commit 3149a5f199

View File

@ -90,7 +90,7 @@ public:
nsresult FinishHostEntry(void); nsresult FinishHostEntry(void);
nsresult CallOnFound(void); nsresult CallOnFound(void);
const char * HostName(void) { return mHostName; } const char * HostName(void) { return mHostName; }
nsresult InitiateDNSLookup(nsDNSService * dnsService); nsresult InitiateDNSLookup(void);
protected: protected:
// Input when creating a nsDNSLookup // Input when creating a nsDNSLookup
@ -274,14 +274,14 @@ nsDNSLookup::CallOnFound(void)
nsresult nsresult
nsDNSLookup::InitiateDNSLookup(nsDNSService * dnsService) nsDNSLookup::InitiateDNSLookup(void)
{ {
nsresult rv = NS_OK; nsresult rv = NS_OK;
#if defined(XP_MAC) #if defined(XP_MAC)
OSErr err; OSErr err;
err = OTInetStringToAddress(dnsService->mServiceRef, (char *)mHostName, (InetHostInfo *)&mInetHostInfo); err = OTInetStringToAddress(gService->mServiceRef, (char *)mHostName, (InetHostInfo *)&mInetHostInfo);
if (err != noErr) if (err != noErr)
rv = NS_ERROR_UNEXPECTED; rv = NS_ERROR_UNEXPECTED;
#endif /* XP_MAC */ #endif /* XP_MAC */
@ -290,8 +290,8 @@ nsDNSLookup::InitiateDNSLookup(nsDNSService * dnsService)
mMsgID = gService->AllocMsgID(); mMsgID = gService->AllocMsgID();
if (mMsgID == 0) if (mMsgID == 0)
return NS_ERROR_UNEXPECTED; return NS_ERROR_UNEXPECTED;
PR_Lock(gService->mThreadLock); // protect against lookup completing before WSAAsyncGetHostByName returns, better to refcount lookup
mLookupHandle = WSAAsyncGetHostByName(dnsService->mDNSWindow, mMsgID, mLookupHandle = WSAAsyncGetHostByName(gService->mDNSWindow, mMsgID,
mHostName, (char *)&mHostEntry.hostEnt, PR_NETDB_BUF_SIZE); mHostName, (char *)&mHostEntry.hostEnt, PR_NETDB_BUF_SIZE);
// check for error conditions // check for error conditions
if (mLookupHandle == nsnull) { if (mLookupHandle == nsnull) {
@ -304,6 +304,7 @@ nsDNSLookup::InitiateDNSLookup(nsDNSService * dnsService)
// the first two calls made to WSAAsyncGetHostByName), to avoid using the handle on // the first two calls made to WSAAsyncGetHostByName), to avoid using the handle on
// those systems. For more info, see bug 23709. // those systems. For more info, see bug 23709.
} }
PR_Unlock(gService->mThreadLock);
#endif /* XP_PC */ #endif /* XP_PC */
#ifdef XP_UNIX #ifdef XP_UNIX
@ -315,7 +316,8 @@ nsDNSLookup::InitiateDNSLookup(nsDNSService * dnsService)
mResult = rv; mResult = rv;
CallOnFound(); CallOnFound();
delete this; if (PR_SUCCESS == status)
delete this; // nsDNSLookup deleted by nsDNSService::Lookup() in failure case
#endif /* XP_UNIX */ #endif /* XP_UNIX */
return rv; return rv;
@ -358,9 +360,7 @@ nsDNSEventProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
LRESULT result = nsnull; LRESULT result = nsnull;
int error = nsnull; int error = nsnull;
nsDNSService * dnsService = (nsDNSService *)GetWindowLong(hWnd, GWL_USERDATA); if ((uMsg >= WM_USER) && (uMsg < WM_USER+128)) {
if ((dnsService != nsnull) && (uMsg >= WM_USER) && (uMsg < WM_USER+128)) {
// dns lookup complete - get error code // dns lookup complete - get error code
error = WSAGETASYNCERROR(lParam); error = WSAGETASYNCERROR(lParam);
@ -370,10 +370,10 @@ nsDNSEventProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
PRBool rv; PRBool rv;
// find matching lookup element // find matching lookup element
PR_Lock(dnsService->mThreadLock); // so we don't collide with thread calling Lookup() PR_Lock(gService->mThreadLock); // so we don't collide with thread calling Lookup()
index = dnsService->mCompletionQueue.Count(); index = gService->mCompletionQueue.Count();
while (index) { while (index) {
lookup = (nsDNSLookup *)dnsService->mCompletionQueue.ElementAt(index-1); lookup = (nsDNSLookup *)gService->mCompletionQueue.ElementAt(index-1);
if (lookup->mMsgID == uMsg) { if (lookup->mMsgID == uMsg) {
break; break;
} }
@ -381,7 +381,7 @@ nsDNSEventProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
} }
if (lookup && (lookup->mMsgID == uMsg)) { if (lookup && (lookup->mMsgID == uMsg)) {
rv = dnsService->mCompletionQueue.RemoveElement(lookup); rv = gService->mCompletionQueue.RemoveElement(lookup);
NS_ASSERTION(rv == PR_TRUE, "error removing dns lookup element."); NS_ASSERTION(rv == PR_TRUE, "error removing dns lookup element.");
lookup->mComplete = PR_TRUE; lookup->mComplete = PR_TRUE;
@ -389,15 +389,15 @@ nsDNSEventProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
lookup->mResult = NS_ERROR_UNKNOWN_HOST; lookup->mResult = NS_ERROR_UNKNOWN_HOST;
(void)lookup->CallOnFound(); (void)lookup->CallOnFound();
dnsService->FreeMsgID(lookup->mMsgID); gService->FreeMsgID(lookup->mMsgID);
delete lookup; // until we implement the dns cache delete lookup; // until we implement the dns cache
} }
result = 0; result = 0;
PR_Unlock(dnsService->mThreadLock); PR_Unlock(gService->mThreadLock);
} }
else if (uMsg == WM_DNS_SHUTDOWN) { else if (uMsg == WM_DNS_SHUTDOWN) {
// dispose DNS EventHandler Window // dispose DNS EventHandler Window
(void) DestroyWindow(dnsService->mDNSWindow); (void) DestroyWindow(gService->mDNSWindow);
PostQuitMessage(0); PostQuitMessage(0);
result = 0; result = 0;
} }
@ -592,8 +592,6 @@ nsDNSService::InitDNSThread(void)
mDNSWindow = CreateWindow(windowClass, "Mozilla:DNSWindow", mDNSWindow = CreateWindow(windowClass, "Mozilla:DNSWindow",
0, 0, 0, 10, 10, NULL, NULL, NULL, NULL); 0, 0, 0, 10, 10, NULL, NULL, NULL, NULL);
(void) SetWindowLong(mDNSWindow, GWL_USERDATA, (long)this);
// sync with Create thread // sync with Create thread
PRMonitor * monitor; PRMonitor * monitor;
PRStatus status; PRStatus status;
@ -810,16 +808,14 @@ nsDNSService::Lookup(nsISupports* clientContext,
(void)listener->OnStartLookup(clientContext, hostName); (void)listener->OnStartLookup(clientContext, hostName);
// initiate async lookup // initiate async lookup
rv = lookup->InitiateDNSLookup(this); rv = lookup->InitiateDNSLookup();
if (rv != NS_OK) { if (rv != NS_OK) {
#if defined(XP_PC) #if defined(XP_PC)
PR_Lock(mThreadLock); PR_Lock(mThreadLock);
mCompletionQueue.RemoveElement(lookup); mCompletionQueue.RemoveElement(lookup);
PR_Unlock(mThreadLock); PR_Unlock(mThreadLock);
#endif #endif
#if !defined(XP_UNIX)
delete lookup; delete lookup;
#endif
} }
return rv; return rv;
} }