1019385 - ABORT: CRT ASSERT f:\dd\vctools\crt_bld\self_64_amd64\crt\src\dbgheap.c(1322) : Assertion failed: _CrtIsValidHeapPointer(pUserData) from js_realloc

Status: RESOLVED DUPLICATE of bug 1030667

(Core :: DOM: Workers, defect)

Description

[Bob Clary [:bc] (inactive)]

Attachment: stack etc.

1. In Aurora Debug build on Windows 7 32bit or 64bit load:

https://www.google.com/maps/place/36%C3%83%C2%83%C3%82%C2%83%C3%83%C2%82%C3%82%C2%82%C3%83%C2%83%C3%82%C2%82%C3%83%C2%82%C3%82%C2%B005%2721.4%22N%2B106%C3%83%C2%83%C3%82%C2%83%C3%83%C2%82%C3%82%C2%82%C3%83%C2%83%C3%82%C2%82%C3%83%C2%82%C3%82%C2%B049%2714.7%22W/%4036.08929%2C-106.82076%2C13173m/data=%213m1%211e3%214m2%213m1%211s0x0:0x0?hl=en

2. ABORT: CRT ASSERT f:\dd\vctools\crt_bld\self_64_amd64\crt\src\dbgheap.c(1322) : Assertion failed: _CrtIsValidHeapPointer(pUserData)

mozjs!js_realloc(void * p = 0x0febb128, unsigned int bytes = 0)+0x38 [c:\work\mozilla\builds\aurora\mozilla\firefox-debug\dist\include\js\utility.h @ 123]
mozjs!AllocateArrayBufferContents(struct JSContext * maybecx = 0x00000000, unsigned int nbytes = 0, void * oldptr = 0x0febb128, unsigned int oldnbytes = 0)+0x3b [c:\work\mozilla\builds\aurora\mozilla\js\src\vm\arraybufferobject.cpp @ 281]
mozjs!JS_ReallocateArrayBufferContents(struct JSContext * maybecx = 0x00000000, unsigned int nbytes = 0, void * oldContents = 0x0febb128, unsigned int oldNbytes = 0)+0x18 [c:\work\mozilla\builds\aurora\mozilla\js\src\vm\arraybufferobject.cpp @ 1079]
xul!mozilla::ArrayBufferBuilder::setCapacity(unsigned int aNewCap = 0)+0x22 [c:\work\mozilla\builds\aurora\mozilla\content\base\src\nsxmlhttprequest.cpp @ 3873]
xul!mozilla::ArrayBufferBuilder::getArrayBuffer(struct JSContext * aCx = 0x026c2f80)+0x2f [c:\work\mozilla\builds\aurora\mozilla\content\base\src\nsxmlhttprequest.cpp @ 3931]
xul!nsXMLHttpRequest::GetResponse(struct JSContext * aCx = 0x026c2f80, class mozilla::ErrorResult * aRv = 0x002feb5c)+0x1cd [c:\work\mozilla\builds\aurora\mozilla\content\base\src\nsxmlhttprequest.cpp @ 970]
xul!nsXMLHttpRequest::GetResponse(struct JSContext * aCx = 0x026c2f80, class JS::MutableHandle<JS::Value> aResult = class JS::MutableHandle<JS::Value>)+0x25 [c:\work\mozilla\builds\aurora\mozilla\content\base\src\nsxmlhttprequest.cpp @ 932]
xul!`anonymous namespace'::EventRunnable::PreDispatch(struct JSContext * aCx = 0x026c2f80, class mozilla::dom::workers::WorkerPrivate * aWorkerPrivate = 0x11f05570)+0x1bc [c:\work\mozilla\builds\aurora\mozilla\dom\workers\xmlhttprequest.cpp @ 1177]
xul!mozilla::dom::workers::WorkerRunnable::Dispatch(struct JSContext * aCx = 0x026c2f80)+0xf2 [c:\work\mozilla\builds\aurora\mozilla\dom\workers\workerrunnable.cpp @ 104]
xul!mozilla::dom::workers::Proxy::HandleEvent(class nsIDOMEvent * aEvent = 0x0f3dcf70)+0x473 [c:\work\mozilla\builds\aurora\mozilla\dom\workers\xmlhttprequest.cpp @ 1074]


Doesn't appear to happen on Windows XP  or on Beta or Nightly.

Comment 1

[Bob Clary [:bc] (inactive)]

The abort isn't 100% reproducible. I used a test that attempted 50 times per bisection attempt to find these ranges. Most occurred within 3-4 attempts but one took 47 attempts. I'll try again with a 100 attempts to be sure the range doesn't change.

Found fix between 20140522173121-20140523172021
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e9b2b72f4e6c&tochange=51b7091471a9
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/05/2014-05-23-mozilla-central-debug/firefox-32.0a1.en-US.debug-win32.installer.exe
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/05/2014-05-24-mozilla-central-debug/firefox-32.0a1.en-US.debug-win32.installer.exe

Found regression between 20140323171614-20140324134644
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=fa098f9fe89c&tochange=5c0673441fc8
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/03/2014-03-24-mozilla-central-debug/firefox-31.0a1.en-US.debug-win32.installer.exe
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/03/2014-03-25-mozilla-central-debug/firefox-31.0a1.en-US.debug-win32.installer.exe

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

The last few frames are in arraybuffer code, cc'ing sfink for his input.

Comment 3

[Bob Clary [:bc] (inactive)]

Note that my latest attempt with 100 iterations per test failed to reliably detect the abort so I would not put much faith in the reported regression/fix ranges.

Comment 4

[Bob Clary [:bc] (inactive)]

Ignore comment 3 for now. I misinterpreted what was happening. As far as I know now, the regression ranges are good. I'll update when the 100 iteration runs complete. Sorry for the spam.

Comment 5

[Bob Clary [:bc] (inactive)]

I repeated the bisection with 100 iterations and found essentially the same range as in comment 1. There does appear still be a problem on trunk after 20140523172021 but it is very difficult to reproduce. I'll try to reproduce it later but it may be a separate issue. Bug 951145 sort of exhibits the same behavior with common occurrences before 2014-05-08 and only once since.

Comment 6

[Andrew McCreight [:mccr8]]

This looks like a dupe of bug 988675.  Somebody should look at that.

Comment 8

[Jeff Walden [:Waldo]]

I'm not sure I agree with the dup here.  That bug's issue is a problem in ASan, only.  This is some sort of issue definitely not with ASan, possibly with the MSVC CRT.  It could perhaps be mishandling |void* p = realloc(nullptr, 0); void* p2 = realloc(p, 0); free(p2);| as well.

And in fact, if I consult the source of the 2010 runtime, it looks like realloc(nullptr, 0) will allocate memory (_heap_alloc converts size to 1 if size is zero), and realloc(thatPointer, 0) will free the memory and return nullptr.  It seems even likelier, now (particularly given a "Special ANSI Requirements" comment in realloc.c), that C89 realloc specified this behavior, and C99 decided it was a bug and fixed it.

All of which suggests, I think, that we probably need to apply extra care to realloc calls that might pass 0 as size.  At least, unless we want to totally give up on supporting non-jemalloc heaps.

And, um.  I just consulted memory/jemalloc/src/src/jemalloc.c.  And it has this in it, for the function that presumably becomes realloc:

void *
je_realloc(void *ptr, size_t size)
{
	void *ret;
	size_t usize JEMALLOC_CC_SILENCE_INIT(0);
	size_t old_usize = 0;
	UNUSED size_t old_rzsize JEMALLOC_CC_SILENCE_INIT(0);

	if (size == 0) {
		if (ptr != NULL) {
			/* realloc(ptr, 0) is equivalent to free(ptr). */
			UTRACE(ptr, 0, 0);
			ifree(ptr);
			return (NULL);
		}
		size = 1;
	}
	...
}

We should fix this to be C99-compliant, for safety.  But if this is really realloc, it seems like (*consults glibc on his system* *finds |2925 if (bytes == 0 && oldmem != NULL) { __libc_free(oldmem); return 0; }| in it*) everyone implements C89 (by hypothesis) realloc semantics.  So we should at least *try* to audit all of our reallocs, to be sure that a zero-sized malloc/realloc can't flow into a realloc(..., 0) that we haven't written to be careful about his issue.  :-(

So, um.  I guess I agree with the dup here.  But there is a real issue to be dealt with in bug 1030667, that we can't just "solve" with an ASan patch.

Comment 9

[Andrew McCreight [:mccr8]]

I believe that the code in memory/jemalloc/ is for jemalloc3 and not the jemalloc we use, which is in memory/mozjemalloc.  The latter has the non-freeing realloc behavior.  It is quite odd that on Windows we're somehow hitting MSVC's realloc code rather than jemalloc, and only in one version.

Comment 10

[Bob Clary [:bc] (inactive)]

FYI, The last time this was seen in the wild was in mid July with Beta/31 just prior to the Firefox 31 release. I just re-tested it with about 100 urls and could not reproduce on Beta/32, Aurora/33 or Nightly/34 on XP 32bit or Win7 32/64bit. Not completely sure this was a dupe, but regardless it is gone at least temporarily.