Closed
Bug 819445
Opened 11 years ago
Closed 11 years ago
~nsHTTPListener() destroys condition variable on which other threads are blocked.
Categories
(Core :: Networking, defect)
Tracking
()
RESOLVED
INVALID
People
(Reporter: ishikawa, Unassigned)
References
Details
TB (and presumably FF) does not check for proper thread synchronization, and when ~nsHTTPListener() is called, it destroys a condition variable on which some other threads are waiting (!) This is bad. Once this is done. Anything goes in terms of thread safety. E.g. http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_cond_init.html > Attempting to destroy a condition variable upon which other threads > are currently blocked results in undefined behavior. I suspect this may be ONE of the evil causes of thread races in TB, which may cause the failures in Bug 814438 - Invalid Read of 4 bytes through bogus pointer in nssCertificate_Destroy (thunderbird) Bug 713624 - Thunderbird locks (deadlock?) on a flaky connection with rss feeds or Bug 819386 - Intermittent Android shutdown crash [@nss_certificate_hash] etc., for example. (But it seems that there *ARE* more places where proper locking/unlocking is missing in TB/FF code :-( And so this problem reported here is not solely responsible for the hard-to-reproduce race issues :-( To witness: the following is a warning from a helgrind thread checker (a tool in valgrind suite.) when TB is run under helgrind. (I tweaked MOZOBJ/mozilla/dist/bin/thunderbird so that it will run thunderbird-bin under valgrind. I ran "make check" to see if there are thread-related problems, and this bugzilla entry is one of the outstanding problem I notice in the session log. ==23074== ---------------------------------------------------------------- ==23074== ==23074== Thread #1: pthread_cond_destroy: destruction of condition variable being waited upon ==23074== at 0x4027A7F: pthread_cond_destroy_WRK (hg_intercepts.c:940) ==23074== by 0x4029781: pthread_cond_destroy@* (hg_intercepts.c:958) ==23074== by 0x47193BA: PR_DestroyCondVar (ptsynch.c:372) ==23074== by 0x5947C40: nsHTTPListener::~nsHTTPListener() (CondVar.h:56) ==23074== by 0x5947D82: nsHTTPListener::Release() (nsNSSCallbacks.cpp:536) ==23074== by 0x603EFCA: nsCOMPtr_base::assign_with_AddRef(nsISupports*) (nsCOMPtr.h:442) ==23074== by 0x4C8569A: nsStreamLoader::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult) (nsCOMPtr.h:624) ==23074== by 0x4C84EA5: nsStreamListenerTee::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult) (nsStreamListenerTee.cpp:49) ==23074== by 0x4D12A45: mozilla::net::nsHttpChannel::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult) (nsHttpChannel.cpp:4994) ==23074== by 0x4C650F8: nsInputStreamPump::OnStateStop() (nsInputStreamPump.cpp:552) ==23074== by 0x4C652AD: nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) (nsInputStreamPump.cpp:374) ==23074== by 0x606E28D: nsInputStreamReadyEvent::Run() (nsStreamUtils.cpp:82) ==23074== { ------------------------ versions (comm-central) ----------------------- cd /home/ishikawa/TB-NEW/TB-3HG/new-src hg identify 99fd4744910d+ tip ----------------------- mozilla subdirectory ----------------------- cd /home/ishikawa/TB-NEW/TB-3HG/new-src/mozilla hg identify 3b71d63eafd5+ tip
| Reporter | ||
Comment 1•11 years ago
|
||
BTW, would it not be great if helgrind can tell us which thread, and in which function is blocking on the cond variable being destroyed? (Maybe helgrind on my PC is running out of memory [linux 32 bits] to record such information since there are so many thread-related warnings. It says after 100 possible errors/warnings, the future errors/warnings may not be detailed.) In order to track down the real causes of races, I am afraid we have to place many proper locking even if there are cases they would slow TB (FF). With the current state of affairs, the sheer volume of warnings from valgrind makes it rather tough to figure out what is really going on. I would post a few more cases where condition variable being waited on is destroyed.
| Reporter | ||
Comment 2•11 years ago
|
||
A few more logs to show where the problem may happen. ==32661== ---------------------------------------------------------------- ==32661== ==32661== Thread #1: pthread_cond_destroy: destruction of condition variable being waited upon ==32661== at 0x4027A7F: pthread_cond_destroy_WRK (hg_intercepts.c:940) ==32661== by 0x4029781: pthread_cond_destroy@* (hg_intercepts.c:958) ==32661== by 0x47193BA: PR_DestroyCondVar (ptsynch.c:372) ==32661== by 0x5947C40: nsHTTPListener::~nsHTTPListener() (CondVar.h:56) ==32661== by 0x5947D82: nsHTTPListener::Release() (nsNSSCallbacks.cpp:536) ==32661== by 0x603EFCA: nsCOMPtr_base::assign_with_AddRef(nsISupports*) (nsCOMPtr.h:442) ==32661== by 0x4C8569A: nsStreamLoader::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult) (nsCOMPtr.h:624) ==32661== by 0x4C84EA5: nsStreamListenerTee::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult) (nsStreamListenerTee.cpp:49) ==32661== by 0x4D12A45: mozilla::net::nsHttpChannel::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult) (nsHttpChannel.cpp:4994) ==32661== by 0x4C650F8: nsInputStreamPump::OnStateStop() (nsInputStreamPump.cpp:552) ==32661== by 0x4C652AD: nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) (nsInputStreamPump.cpp:374) ==32661== by 0x606E28D: nsInputStreamReadyEvent::Run() (nsStreamUtils.cpp:82) ==32661== ==32661== ---------------------------------------------------------------- ==32661== ==32661== Thread #30: pthread_cond_destroy: destruction of condition variable being waited upon ==32661== at 0x4027A7F: pthread_cond_destroy_WRK (hg_intercepts.c:940) ==32661== by 0x4029781: pthread_cond_destroy@* (hg_intercepts.c:958) ==32661== by 0x47193BA: PR_DestroyCondVar (ptsynch.c:372) ==32661== by 0x5947C40: nsHTTPListener::~nsHTTPListener() (CondVar.h:56) ==32661== by 0x5947D82: nsHTTPListener::Release() (nsNSSCallbacks.cpp:536) ==32661== by 0x594755A: nsNSSHttpRequestSession::~nsNSSHttpRequestSession() (nsCOMPtr.h:410) ==32661== by 0x477207D: CERT_CheckOCSPStatus (ocsp.c:3509) ==32661== by 0x47748E1: CERT_VerifyCert (certvfy.c:1330) ==32661== by 0x4774981: CERT_VerifyCertNow (certvfy.c:1365) ==32661== by 0x595642D: mozilla::psm::(anonymous namespace)::AuthCertificate(mozilla::psm::TransportSecurityInfo*, CERTCertificateStr*) (SSLServerCertVerification.cpp:646) ==32661== by 0x5956759: mozilla::psm::(anonymous namespace)::SSLServerCertVerificationJob::Run() (SSLServerCertVerification.cpp:1046) ==32661== by 0x60866C3: nsThreadPool::Run() (nsThreadPool.cpp:194) ==32661== The following seems to be the same as first one. ==740== ---------------------------------------------------------------- ==740== ==740== Thread #1: pthread_cond_destroy: destruction of condition variable being waited upon ==740== at 0x4027A7F: pthread_cond_destroy_WRK (hg_intercepts.c:940) ==740== by 0x4029781: pthread_cond_destroy@* (hg_intercepts.c:958) ==740== by 0x47193BA: PR_DestroyCondVar (ptsynch.c:372) ==740== by 0x5947C40: nsHTTPListener::~nsHTTPListener() (CondVar.h:56) ==740== by 0x5947D82: nsHTTPListener::Release() (nsNSSCallbacks.cpp:536) ==740== by 0x603EFCA: nsCOMPtr_base::assign_with_AddRef(nsISupports*) (nsCOMPtr.h:442) ==740== by 0x4C8569A: nsStreamLoader::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult) (nsCOMPtr.h:624) ==740== by 0x4D12A45: mozilla::net::nsHttpChannel::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult) (nsHttpChannel.cpp:4994) ==740== by 0x4C650F8: nsInputStreamPump::OnStateStop() (nsInputStreamPump.cpp:552) ==740== by 0x4C652AD: nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) (nsInputStreamPump.cpp:374) ==740== by 0x606E28D: nsInputStreamReadyEvent::Run() (nsStreamUtils.cpp:82) ==740== by 0x6083B4B: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:642) ==740== PKIX related context. ==740== ---------------------------------------------------------------- ==740== ==740== Thread #39: pthread_cond_destroy: destruction of condition variable being waited upon ==740== at 0x4027A7F: pthread_cond_destroy_WRK (hg_intercepts.c:940) ==740== by 0x4029781: pthread_cond_destroy@* (hg_intercepts.c:958) ==740== by 0x47193BA: PR_DestroyCondVar (ptsynch.c:372) ==740== by 0x5947C40: nsHTTPListener::~nsHTTPListener() (CondVar.h:56) ==740== by 0x5947D82: nsHTTPListener::Release() (nsNSSCallbacks.cpp:536) ==740== by 0x594755A: nsNSSHttpRequestSession::~nsNSSHttpRequestSession() (nsCOMPtr.h:410) ==740== by 0x4803FFA: PKIX_PL_Object_DecRef (pkix_pl_object.c:891) ==740== by 0x47C1BF4: pkix_OcspChecker_CheckExternal (pkix_ocspchecker.c:341) ==740== by 0x47C270B: PKIX_RevocationChecker_Check (pkix_revocationchecker.c:390) ==740== by 0x47D404D: pkix_CheckChain (pkix_validate.c:823) ==740== by 0x47D6EDF: pkix_Build_ValidateEntireChain (pkix_build.c:1307) ==740== by 0x47DAD24: pkix_BuildForwardDepthFirstSearch (pkix_build.c:2530) ==740== From within JavaScript??? ==1345== ---------------------------------------------------------------- ==1345== ==1345== Thread #1: pthread_cond_destroy: destruction of condition variable being waited upon ==1345== at 0x4027A7F: pthread_cond_destroy_WRK (hg_intercepts.c:940) ==1345== by 0x4029781: pthread_cond_destroy@* (hg_intercepts.c:958) ==1345== by 0x47193BA: PR_DestroyCondVar (ptsynch.c:372) ==1345== by 0x5947C40: nsHTTPListener::~nsHTTPListener() (CondVar.h:56) ==1345== by 0x5947D82: nsHTTPListener::Release() (nsNSSCallbacks.cpp:536) ==1345== by 0x603EFCA: nsCOMPtr_base::assign_with_AddRef(nsISupports*) (nsCOMPtr.h:442) ==1345== by 0x57DE619: XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) (XPCWrappedNative.cpp:3081) ==1345== by 0x57E5ED7: XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) (XPCWrappedNativeJSOps.cpp:1488) ==1345== by 0x6768315: js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) (jscntxtinlines.h:364) ==1345== by 0x6759A10: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:2334) ==1345== by 0x6767339: js::RunScript(JSContext*, JS::Handle<JSScript*>, js::StackFrame*) (jsinterp.cpp:326) ==1345== by 0x67684D2: js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) (jsinterp.cpp:384) ==1345== Same as the earlier PKIX context example ==1345== ---------------------------------------------------------------- ==1345== ==1345== Thread #29: pthread_cond_destroy: destruction of condition variable being waited upon ==1345== at 0x4027A7F: pthread_cond_destroy_WRK (hg_intercepts.c:940) ==1345== by 0x4029781: pthread_cond_destroy@* (hg_intercepts.c:958) ==1345== by 0x47193BA: PR_DestroyCondVar (ptsynch.c:372) ==1345== by 0x5947C40: nsHTTPListener::~nsHTTPListener() (CondVar.h:56) ==1345== by 0x5947D82: nsHTTPListener::Release() (nsNSSCallbacks.cpp:536) ==1345== by 0x594755A: nsNSSHttpRequestSession::~nsNSSHttpRequestSession() (nsCOMPtr.h:410) ==1345== by 0x4803FFA: PKIX_PL_Object_DecRef (pkix_pl_object.c:891) ==1345== by 0x47C1BF4: pkix_OcspChecker_CheckExternal (pkix_ocspchecker.c:341) ==1345== by 0x47C270B: PKIX_RevocationChecker_Check (pkix_revocationchecker.c:390) ==1345== by 0x47D404D: pkix_CheckChain (pkix_validate.c:823) ==1345== by 0x47D6EDF: pkix_Build_ValidateEntireChain (pkix_build.c:1307) ==1345== by 0x47DAD24: pkix_BuildForwardDepthFirstSearch (pkix_build.c:2530) ==1345== Shutdown context: ==1597== ---------------------------------------------------------------- ==1597== ==1597== Thread #1: pthread_cond_destroy: destruction of condition variable being waited upon ==1597== at 0x4027A7F: pthread_cond_destroy_WRK (hg_intercepts.c:940) ==1597== by 0x4029781: pthread_cond_destroy@* (hg_intercepts.c:958) ==1597== by 0x47193BA: PR_DestroyCondVar (ptsynch.c:372) ==1597== by 0x5947C40: nsHTTPListener::~nsHTTPListener() (CondVar.h:56) ==1597== by 0x5947D82: nsHTTPListener::Release() (nsNSSCallbacks.cpp:536) ==1597== by 0x603EFCA: nsCOMPtr_base::assign_with_AddRef(nsISupports*) (nsCOMPtr.h:442) ==1597== by 0x5947AC7: nsCancelHTTPDownloadEvent::Run() (nsCOMPtr.h:624) ==1597== by 0x6083B4B: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:642) ==1597== by 0x60439A4: NS_ProcessNextEvent_P(nsIThread*, bool) (nsThreadUtils.cpp:221) ==1597== by 0x6084C26: nsThread::Shutdown() (nsThread.cpp:484) ==1597== by 0x59A7382: mozilla::storage::(anonymous namespace)::AsyncCloseConnection::Run() (mozStorageConnection.cpp:369) ==1597== by 0x6083B4B: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:642) ==1597== Another java script context? ==1702== ---------------------------------------------------------------- ==1702== ==1702== Thread #1: pthread_cond_destroy: destruction of condition variable being waited upon ==1702== at 0x4027A7F: pthread_cond_destroy_WRK (hg_intercepts.c:940) ==1702== by 0x4029781: pthread_cond_destroy@* (hg_intercepts.c:958) ==1702== by 0x47193BA: PR_DestroyCondVar (ptsynch.c:372) ==1702== by 0x5947C40: nsHTTPListener::~nsHTTPListener() (CondVar.h:56) ==1702== by 0x5947D82: nsHTTPListener::Release() (nsNSSCallbacks.cpp:536) ==1702== by 0x603EFCA: nsCOMPtr_base::assign_with_AddRef(nsISupports*) (nsCOMPtr.h:442) ==1702== by 0x57DE619: XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) (XPCWrappedNative.cpp:3081) ==1702== by 0x57E5ED7: XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) (XPCWrappedNativeJSOps.cpp:1488) ==1702== by 0x6768315: js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) (jscntxtinlines.h:364) ==1702== by 0x6759A10: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:2334) ==1702== by 0x6767339: js::RunScript(JSContext*, JS::Handle<JSScript*>, js::StackFrame*) (jsinterp.cpp:326) ==1702== by 0x67684D2: js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) (jsinterp.cpp:384) ==1702== Similar to the two earlier PKIX related context. ==1702== ---------------------------------------------------------------- ==1702== ==1702== Thread #28: pthread_cond_destroy: destruction of condition variable being waited upon ==1702== at 0x4027A7F: pthread_cond_destroy_WRK (hg_intercepts.c:940) ==1702== by 0x4029781: pthread_cond_destroy@* (hg_intercepts.c:958) ==1702== by 0x47193BA: PR_DestroyCondVar (ptsynch.c:372) ==1702== by 0x5947C40: nsHTTPListener::~nsHTTPListener() (CondVar.h:56) ==1702== by 0x5947D82: nsHTTPListener::Release() (nsNSSCallbacks.cpp:536) ==1702== by 0x594755A: nsNSSHttpRequestSession::~nsNSSHttpRequestSession() (nsCOMPtr.h:410) ==1702== by 0x4803FFA: PKIX_PL_Object_DecRef (pkix_pl_object.c:891) ==1702== by 0x47C1BF4: pkix_OcspChecker_CheckExternal (pkix_ocspchecker.c:341) ==1702== by 0x47C270B: PKIX_RevocationChecker_Check (pkix_revocationchecker.c:390) ==1702== by 0x47D404D: pkix_CheckChain (pkix_validate.c:823) ==1702== by 0x47D6EDF: pkix_Build_ValidateEntireChain (pkix_build.c:1307) ==1702== by 0x47DAD24: pkix_BuildForwardDepthFirstSearch (pkix_build.c:2530) ==1702== Similar to an earlier example. ==2466== ---------------------------------------------------------------- ==2466== ==2466== Thread #1: pthread_cond_destroy: destruction of condition variable being waited upon ==2466== at 0x4027A7F: pthread_cond_destroy_WRK (hg_intercepts.c:940) ==2466== by 0x4029781: pthread_cond_destroy@* (hg_intercepts.c:958) ==2466== by 0x47193BA: PR_DestroyCondVar (ptsynch.c:372) ==2466== by 0x5947C40: nsHTTPListener::~nsHTTPListener() (CondVar.h:56) ==2466== by 0x5947D82: nsHTTPListener::Release() (nsNSSCallbacks.cpp:536) ==2466== by 0x603EFCA: nsCOMPtr_base::assign_with_AddRef(nsISupports*) (nsCOMPtr.h:442) ==2466== by 0x4C8569A: nsStreamLoader::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult) (nsCOMPtr.h:624) ==2466== by 0x4D04866: mozilla::net::HttpBaseChannel::DoNotifyListener() (HttpBaseChannel.cpp:1463) ==2466== by 0x57DE619: XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) (XPCWrappedNative.cpp:3081) ==2466== by 0x57E5ED7: XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) (XPCWrappedNativeJSOps.cpp:1488) ==2466== by 0x6768315: js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) (jscntxtinlines.h:364) ==2466== by 0x6759A10: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:2334) ==2466==
| Reporter | ||
Comment 3•11 years ago
|
||
I wonder if such destruction of condition variable while other tasks
are waiting on it lead to the following problematic log and similar.
e.g.
==1980== ----------------------------------------------------------------
==1980==
==1980== Thread #1: pthread_cond_destroy: destruction of unknown cond var
==1980== at 0x4027A7F: pthread_cond_destroy_WRK (hg_intercepts.c:940)
==1980== by 0x4029781: pthread_cond_destroy@* (hg_intercepts.c:958)
==1980== by 0x47193BA: PR_DestroyCondVar (ptsynch.c:372)
==1980== by 0x47A555A: IssuerCache_Destroy (crl.c:1103)
==1980== by 0x47A5620: FreeIssuer (crl.c:1243)
==1980== by 0x403FAE9: PL_HashTableEnumerateEntries (plhash.c:374)
==1980== by 0x47A56CB: ShutdownCRLCache (crl.c:1322)
==1980== by 0x476B02F: nss_Shutdown (nssinit.c:1086)
==1980== by 0x476B168: NSS_Shutdown (nssinit.c:1149)
==1980== by 0x594E456: nsNSSComponent::ShutdownNSS() (nsNSSComponent.cpp:1866)
==1980== by 0x594EE91: nsNSSComponent::DoProfileBeforeChange(nsISupports*) (nsNSSComponent.cpp:2549)
==1980== by 0x594F08B: nsNSSComponent::Observe(nsISupports*, char const*, unsigned short const*) (nsNSSComponent.cpp:2176)
==1980==
{| Reporter | ||
Updated•11 years ago
|
Summary: ~nsHTTPListener() destroys conditon variable on which other threads are blocked. → ~nsHTTPListener() destroys condition variable on which other threads are blocked.
| Reporter | ||
Comment 4•11 years ago
|
||
(In reply to ISHIKAWA, chiaki from comment #3) > I wonder if such destruction of condition variable while other tasks > are waiting on it lead to the following problematic log and similar. > The original report still stands, but the above question seem to be due to false-positive of helgrind. See https://bugs.kde.org/show_bug.cgi?id=307082 HG false positive: pthread_cond_destroy: destruction of unknown cond var helgrind was not handling pthread_cond_init() correctly, and thus cond var was considered unknown. In the kde bug entry, a patch was posted (created by me) and is being reviewed. I am using the patch to test thunderbird under helgrind and "destruction of unknown cond var" messages do not seem to be generated very often. So the question in comment 3 can be ignored for now. The original question of "pthread_cond_destroy: destruction of condition variable being waited upon" is still being investigated. I am trying to figure out how to do the following: >BTW, would it not be great if helgrind can tell us which thread, and in which >function is blocking on the cond variable being destroyed? At least, I think I can print the thread id of the waiting task(s) without much difficulty, so intend to find out what tasks are waiting on the destroyed cond variables.
| Reporter | ||
Comment 5•11 years ago
|
||
A good news. TB source code was OK. It seems that stock valgrind/helgrind version 3.8.1 produced false positives for pthread_cond_destroy(). I produced a patch to fix the problem and no longer see the warning (also no warnings from my audited source code to print error warnings if pthread_cond_destroy() fails.) Details is in Bug 819599, ( "~nsEventQueue() destroys condition variable on which other threads are blocked." specifically comment https://bugzilla.mozilla.org/show_bug.cgi?id=819599#c7 So I think we can safely close this for now. Sorry for the noise.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → INVALID
Comment 6•11 years ago
|
||
Did you run the tests with nsHttpTransaction::Release annotated as
shown below? Without that, all thread checking tools will show false
errors since they do not understand threadsafe refcounting and
destructors.
diff --git a/netwerk/protocol/http/nsHttpTransaction.cpp b/netwerk/protocol/http/nsHttpTransaction.cpp
--- a/netwerk/protocol/http/nsHttpTransaction.cpp
+++ b/netwerk/protocol/http/nsHttpTransaction.cpp
@@ -24,16 +24,20 @@
#include "nsMultiplexInputStream.h"
#include "nsStringStream.h"
#include "nsComponentManagerUtils.h" // do_CreateInstance
#include "nsServiceManagerUtils.h" // do_GetService
#include "nsIHttpActivityObserver.h"
#include "nsSocketTransportService2.h"
+#ifdef MOZ_VALGRIND
+# include <valgrind/helgrind.h>
+#endif
+
using namespace mozilla;
//-----------------------------------------------------------------------------
#ifdef DEBUG
// defined by the socket transport service while active
extern PRThread *gSocketThread;
@@ -1597,23 +1601,29 @@ NS_IMPL_THREADSAFE_ADDREF(nsHttpTransact
NS_IMETHODIMP_(nsrefcnt)
nsHttpTransaction::Release()
{
nsrefcnt count;
NS_PRECONDITION(0 != mRefCnt, "dup release");
count = NS_AtomicDecrementRefcnt(mRefCnt);
NS_LOG_RELEASE(this, count, "nsHttpTransaction");
+ // HGANN 018 BEGIN REQD_ANN thread-safe-ref-counting
if (0 == count) {
+ ANNOTATE_HAPPENS_AFTER(&mRefCnt);
+ ANNOTATE_HAPPENS_BEFORE_FORGET_ALL(&mRefCnt);
mRefCnt = 1; /* stablize */
// it is essential that the transaction be destroyed on the consumer
// thread (we could be holding the last reference to our consumer).
DeleteSelfOnConsumerThread();
return 0;
+ } else {
+ ANNOTATE_HAPPENS_BEFORE(&mRefCnt);
}
+ // HGANN 018 END
return count;
}
NS_IMPL_THREADSAFE_QUERY_INTERFACE2(nsHttpTransaction,
nsIInputStreamCallback,
nsIOutputStreamCallback)
//-----------------------------------------------------------------------------| Reporter | ||
Comment 7•11 years ago
|
||
(In reply to Julian Seward from comment #6) > Did you run the tests with nsHttpTransaction::Release annotated as > shown below? Without that, all thread checking tools will show false > errors since they do not understand threadsafe refcounting and > destructors. > Thank you for you comment. Actually, I took a previous patch posted to the following bugzilla entry, Bug 551155 - mark up Fx codebase with race-detector annotations (edit) and modified it slightly so that the patch can be applied cleanly to comm-central source files: https://bugzilla.mozilla.org/attachment.cgi?id=678284 " Re-created patch against later C-C mozilla subdirectory (with USEHELGRIND macro) " I notice that my patch doesn't include valgrind/valgrind.h directory (not sure if this is fatal or not. I suspect that this causes a compile-time warning.) (Also, maybe I should use MOZ_HELGRIND for #if/#endif condition?) The problem with helgrind 3.8.1 was that it did not handle pthread_cond_init() correctly (it did not handle it at all!), thus the cond variable was not known to the rest of valgrind. This is why we saw "unknown cond var", and I also fixed the problem of helgrind prematurely destroyed internal data structure to handle cond var when pthread_cond_destroy() would return EBUSY and retain the cond var. (The details are in http://bugsfiles.kde.org/attachment.cgi?id=75985 "HG false positive: pthread_cond_destroy: destruction of unknown cond var" ) After applying the patch in the above URL at kde.org to helgrind, I did not see any similar warnings (and I also added the check to verify the code for pthread_cond_destroy() to make doubly sure that pthread_cond_destroy() is returning a non-zero value. So far, the log looks good.) TIA
You need to log in
before you can comment on or make changes to this bug.
Description
•