User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9a1) Gecko/20060228 Firefox/1.6a1 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9a1) Gecko/20060624 Minefield/3.0a1 Ad Muncher (http://www.admuncher.com/) is a small ad-blocking application which simply acts as a proxy for HTTP(S) traffic. I've been using it for about a year now. Crashes don't occur in the following build: Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9a1) Gecko/20060228 Firefox/1.6a1 - Build ID: 2006022805 This one of the last non-Places trunk builds, I've recently wanted to switch to current trunk builds and that's how I discovered this crash. It has been confirmed on other PCs. When Ad Muncher is not running - everything is OK. I don't think it's Ad Muncher's issue, because it works OK in some previous trunks (unfortunately I can't give any approximate regression window - I haven't been regular trunk user since February, but I'm sure that it worked OK in selected builds from previous months...). Reproducible: Always Steps to Reproduce: 1. Launch Ad Muncher 2. Open any "https://" link in Minefield Actual Results: Firefox crashes, Talkback kicks in, but it doesn't work on my PC (doesn't send anything to the server so I can't give you any IDs). Expected Results: Open the page as usual. I don't know if this will be useful but I'm attaching Dr Watson log of one of such crashes...

imm32.dll is most likely culprit i just have too put it in the firefox folder and crash on start

If you can install Talkback and get a Talkback ID for such a crash, that would be helpful. Also, if you can use nightlies to narrow the regression window as small as possible, that too would be helpful.

I am the lead developer of Ad Muncher, Murray Hurps. Firstly thank you very much for your help with this issue. Using Firefox 2.0b1 I generated talkback ID TB20936818H when attempting to check for updates from within Firefox. Ad Muncher's filtering works by doing in-memory patching of the "WSAStartup", "connect", "getpeername" and "getsockname" functions in wsock32.dll or ws2_32.dll (with preference for the latter if present). "WSAStartup" is patched purely for startup purposes, and no parameters or return values are altered. "connect" is patched in order to change the connection target from the original to a localhost address, which the Ad Muncher program is listening on. Ad Muncher then connects out to the original target and relays information, with filtering, between the two connections. "getpeername" and "getsockname" are patched to improve transparency of the system. The original peer is returned, and the original local name. From my analysis so far it looks like the problem only occurs if Ad Muncher redirects the connection Firefox makes to a local address on startup. Starting Ad Muncher after this connection is made avoids the problem, and modifying Ad Muncher not to redirect loopback connections also prevented the problem (but we would prefer not to use this as a long-term fix). I traced through Ad Muncher's patches and all patch functions and winsock APIs completed without fault, the error occurs after returning to Firefox code. Ad Muncher can be downloaded from: http://www.admuncher.com/beta.pl If there is any more information I can provide, or if anyone can suggest a change to Ad Muncher's patching behavior that might better work with Firefox, please let me know. Thanks again.

reporter: thank you very much for including the relevant application in your filing. this is rare and we really appreciate it. the connection is used by nss Incident ID: 20936818 Stack Signature PL_DHashTableOperate 8581f7b4 Product ID Firefox2 Build ID 2006071020 Trigger Time 2006-07-14 02:28:17.0 Platform Win32 Operating System Windows NT 5.0 build 2195 Module xpcom_core.dll + (0000131d) URL visited User Comments Checking for updates in Firefox with Ad Muncher loaded Since Last Crash 9 sec Total Uptime 21 sec Trigger Reason Access violation Source File, Line No. c:/builds/tinderbox/Fx-Mozilla1.8-release/WINNT_5.2_Depend/mozilla/xpcom/build/pldhash.c, line 547 Stack Trace PL_DHashTableOperate [c:/builds/tinderbox/Fx-Mozilla1.8-release/WINNT_5.2_Depend/mozilla/xpcom/build/pldhash.c, line 547] nsCStringHashSetSuper::GetEntry [c:/builds/tinderbox/Fx-Mozilla1.8-release/WINNT_5.2_Depend/mozilla/xpcom/ds/nsHashSets.cpp, line 43] nsSSLIOLayerAddToSocket [c:/builds/tinderbox/Fx-Mozilla1.8-release/WINNT_5.2_Depend/mozilla/security/manager/ssl/src/nsNSSIOLayer.cpp, line 2574] nsSSLIOLayerNewSocket [c:/builds/tinderbox/Fx-Mozilla1.8-release/WINNT_5.2_Depend/mozilla/security/manager/ssl/src/nsNSSIOLayer.cpp, line 1410] nsSSLSocketProvider::NewSocket [c:/builds/tinderbox/Fx-Mozilla1.8-release/WINNT_5.2_Depend/mozilla/security/manager/ssl/src/nsSSLSocketProvider.cpp, line 72] nsSocketTransport::BuildSocket [c:/builds/tinderbox/Fx-Mozilla1.8-release/WINNT_5.2_Depend/mozilla/netwerk/base/src/nsSocketTransport2.cpp, line 988] nsSocketTransport::InitiateSocket [c:/builds/tinderbox/Fx-Mozilla1.8-release/WINNT_5.2_Depend/mozilla/netwerk/base/src/nsSocketTransport2.cpp, line 1090] nsSocketTransport::OnSocketEvent [c:/builds/tinderbox/Fx-Mozilla1.8-release/WINNT_5.2_Depend/mozilla/netwerk/base/src/nsSocketTransport2.cpp, line 1406] nsUrlClassifierDBServiceWorker::AddRef [c:/builds/tinderbox/Fx-Mozilla1.8-release/WINNT_5.2_Depend/mozilla/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp, line 193] 0x81088b56

Kai, could you tell if this bug is in Necko (networking library), PSM, or XPCOM (pldhash.c)?

i can confirm above mentioned crashes as described in bugs https://bugzilla.mozilla.org/show_bug.cgi?id=333540 and https://bugzilla.mozilla.org/show_bug.cgi?id=338767

*** Bug 344933 has been marked as a duplicate of this bug. ***

Mozillazine forum thread about this can be found here: http://forums.mozillazine.org/viewtopic.php?p=2376096 It has more information and talkback IDs.

*** Bug 338767 has been marked as a duplicate of this bug. ***

This regressed on trunk between 2006-04-04 and 2006-04-05. It regressed on 1.8.1 branch between 2006-04-13 and 2006-04-15. So most likely a regression from bug 111384.

I understand what's going on and will attach a fix shortly.

*** Bug 344934 has been marked as a duplicate of this bug. ***

As Darin mentioned to me recently in a different bug, PR_NewPollableEvent might fail on Windows - which was not clear to me. (The new SSL code that requires an pollable event to be present had been introduced with the patch for bug 111384) While I played with AdMuncher installed, I noticed that the SSL code's call to this function indeed fails sometimes (to my surprise it only fails about 50% of attempts). Because the new SSL code had been implement in a way to ultimately require a pollable event, the error checking is currently less than perfect, and it causes the application to crash when we attempt to use it. I can easily add checks that will prevent us from crashing, should pollable event creation fail. Unfortunately this would not be a good solution, because it would disable all https / SSL activity. I worked on fallback code, that will make SSL work, even when PR_NewPollableEvent fails.

Kai, maybe you also could/want to take a look at bug 339292? That is a crash with ad muncer on the 1.8.0.x branch.

Patch v1, tested on Windows, with and without AdMunch, with both OCSP enabled and disabled.

Same as Patch v1, but this version ignores whitespace changes, it is easier to review.

Comment on attachment 231799 [details] [diff] [review] Patch v1 - ignoring whitespace changes Bob, can you please review? I hope the comments in the patch are sufficient to explain the fallback strategy.

Kai, please cite the bug and comment numbers for Darin's remark, which you mentioned above in comment 14.

*** Bug 333540 has been marked as a duplicate of this bug. ***

(In reply to comment #19) > Kai, please cite the bug and comment numbers for Darin's remark, which you > mentioned above in comment 14. bug 336944 comment 16

Comment on attachment 231799 [details] [diff] [review] Patch v1 - ignoring whitespace changes r+, but please fix the indentation of some of the } functions where new if's have been added and changed the basic level of some of the untouched code blocks.

(In reply to comment #22) > r+, but please fix the indentation of some of the } functions where new if's > have been added and changed the basic level of some of the untouched code > blocks. Thanks for the r+. The indentation is fixed in the "real" patch v1 attached to this bug - you reviewed a "diff -w" version.

Patch v1 was produced agaginst on older snapshot of the 1.8 branch, it did not apply cleanly to the trunk. This Patch v2 produces the same code as v1. I applies cleanly to both trunk and current 1.8 branch.

Patch that applies cleanly to current trunk and ignores whitespace changes. Equivalent to v1, carrying forward r=rrelyea

Fixed on trunk. Please test tomorrow's (!) nightly trunk build and report whether the bug is fixed for you. Thanks! (Clearing blocking 1.9 flag, we want this fix in 1.8.1)

*** Bug 347855 has been marked as a duplicate of this bug. ***

Are you planning to request approval for 1.8.1? Beta2 deadlines are approaching.

Comment on attachment 232586 [details] [diff] [review] Patch v2 - ignoring whitespace changes (In reply to comment #28) > Are you planning to request approval for 1.8.1? Beta2 deadlines are > approaching. ok, I'm hereby requesting 1.8.1 approval. I had hoped to get some feedback from other testers first. WADA, or other people who saw that crash: does today's nightly trunk build work for you?

*** Bug 340216 has been marked as a duplicate of this bug. ***

(In reply to comment #29) > (From update of attachment 232586 [details] [diff] [review] [edit]) > (In reply to comment #28) > > Are you planning to request approval for 1.8.1? Beta2 deadlines are > > approaching. > > ok, I'm hereby requesting 1.8.1 approval. > > I had hoped to get some feedback from other testers first. > WADA, or other people who saw that crash: does today's nightly trunk build work > for you? > It looks like it is working. No more instant crash at start-up with Ad Muncher for me (to a HTTPS page).

This fix has solved all crashes I was experiencing with FF when Ad-Muncher was also running! Great work Kai! Just need it to land on 2.0 branch now.

Comment on attachment 232586 [details] [diff] [review] Patch v2 - ignoring whitespace changes a=dbaron on behalf of drivers. Please check in to MOZILLA_1_8_BRANCH and add the fixed1.8.1 keyword once you have done so.

> Please check in to MOZILLA_1_8_BRANCH and add > the fixed1.8.1 keyword once you have done so. done

verified per comment #31

*** Bug 345904 has been marked as a duplicate of this bug. ***