Closed Bug 1143223 Opened 8 years ago Closed 8 years ago

Intermittent ASAN test_https_fetch_cloned_response.html | application terminated with exit code 1 after SUMMARY: AddressSanitizer: SEGV nsCOMPtr.h:296 ~nsCOMPtr_base

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Version:
39 Branch
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla39
Tracking Flags:
Tracking Status
firefox37 --- unaffected
firefox38 --- unaffected
firefox39 --- fixed
firefox-esr31 --- unaffected

People

(Reporter: KWierso, Assigned: bkelly)

References

(Blocks 1 open bug)

Details

(Keywords: intermittent-failure)

Attachments

(2 files, 1 obsolete file)

Teach Cache ReadStream not to AddRef() itself in its destructor. r=ehsan
48.14 KB, patch
ehsan.akhgari
: review-
Details | Diff | Splinter Review
interdiff 001 address review feedback
5.31 KB, patch
ehsan.akhgari
: review+
Details | Diff | Splinter Review 
13:55:34 INFO - 413 INFO TEST-START | dom/workers/test/serviceworkers/test_https_fetch_cloned_response.html
13:55:35 INFO - ASAN:SIGSEGV
13:55:35 INFO - =================================================================
13:55:35 INFO - ==2073==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f9403a2ca65 sp 0x7f93e31f8460 bp 0x7f93e31f8490 T75)
13:55:37 INFO - #0 0x7f9403a2ca64 in ~nsCOMPtr_base /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dom/cache/../../dist/include/nsCOMPtr.h:296
13:55:37 INFO - #1 0x7f9403a2ca64 in ~ReadStream /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/cache/ReadStream.cpp:438
13:55:37 INFO - #2 0x7f9403a2ca64 in ~ReadStreamChild /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/cache/ReadStream.cpp:67
13:55:37 INFO - #3 0x7f9403a2ca64 in (anonymous namespace)::ReadStreamChild::~ReadStreamChild() /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/cache/ReadStream.cpp:63
13:55:37 INFO - #4 0x7f9403a2238c in mozilla::dom::cache::ReadStream::Release() /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/cache/ReadStream.cpp:289
13:55:37 INFO - #5 0x7f93ffe07fd6 in operator= /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/xpcom/io/../../dist/include/nsCOMPtr.h:546
13:55:37 INFO - #6 0x7f93ffe07fd6 in nsAStreamCopier::Process() /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/io/nsStreamUtils.cpp:340
13:55:37 INFO - #7 0x7f93ffe047dc in Run /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/io/nsStreamUtils.cpp:417
13:55:37 INFO - #8 0x7f93ffe047dc in non-virtual thunk to nsAStreamCopier::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/xpcom/io/Unified_cpp_xpcom_io1.cpp:482
13:55:37 INFO - #9 0x7f93ffe4614a in nsThreadPool::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThreadPool.cpp:225
13:55:37 INFO - #10 0x7f93ffe464fc in non-virtual thunk to nsThreadPool::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/xpcom/threads/Unified_cpp_xpcom_threads0.cpp:239
13:55:37 INFO - #11 0x7f93ffe40514 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThread.cpp:855
13:55:37 INFO - #12 0x7f93ffea227a in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:265
13:55:37 INFO - #13 0x7f94006e4baf in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/glue/MessagePump.cpp:339
13:55:37 INFO - #14 0x7f940068d40c in RunInternal /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:233
13:55:37 INFO - #15 0x7f940068d40c in RunHandler /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:226
13:55:37 INFO - #16 0x7f940068d40c in MessageLoop::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:200
13:55:37 INFO - #17 0x7f93ffe3cf85 in nsThread::ThreadFunc(void*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThread.cpp:356
13:55:37 INFO - #18 0x7f9417b79135 in _pt_root /builds/slave/m-in-l64-asan-0000000000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:212
13:55:37 INFO - #19 0x7f941b094e99 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x7e99)
13:55:37 INFO - #20 0x7f941a1a42ec (/lib/x86_64-linux-gnu/libc.so.6+0xf42ec)
13:55:37 INFO - AddressSanitizer can not provide additional info.
13:55:37 INFO - SUMMARY: AddressSanitizer: SEGV /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dom/cache/../../dist/include/nsCOMPtr.h:296 ~nsCOMPtr_base
13:55:37 INFO - Thread T75 (StreamTrans #7) created by T62 (DOM Worker) here:
13:55:37 INFO - #0 0x45ea95 in __interceptor_pthread_create /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:175
13:55:37 INFO - #1 0x7f9417b75abd in _PR_CreateThread /builds/slave/m-in-l64-asan-0000000000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:453
13:55:37 INFO - #2 0x7f9417b7563a in PR_CreateThread /builds/slave/m-in-l64-asan-0000000000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:544
13:55:37 INFO - #3 0x7f93ffe3e4bb in nsThread::Init() /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThread.cpp:467
13:55:37 INFO - #4 0x7f93ffe43d1c in nsThreadManager::NewThread(unsigned int, unsigned int, nsIThread**) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThreadManager.cpp:352
13:55:37 INFO - #5 0x7f93ffe45238 in nsThreadPool::PutEvent(nsIRunnable*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThreadPool.cpp:101
13:55:37 INFO - #6 0x7f93ffe46979 in nsThreadPool::Dispatch(nsIRunnable*, unsigned int) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThreadPool.cpp:266
13:55:37 INFO - #7 0x7f94000654b0 in nsStreamTransportService::Dispatch(nsIRunnable*, unsigned int) /builds/slave/m-in-l64-asan-0000000000000000/build/src/netwerk/base/nsStreamTransportService.cpp:518
13:55:37 INFO - #8 0x7f93ffdfb260 in PostContinuationEvent_Locked /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/io/nsStreamUtils.cpp:449
13:55:37 INFO - #9 0x7f93ffdfb260 in PostContinuationEvent /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/io/nsStreamUtils.cpp:440
13:55:37 INFO - #10 0x7f93ffdfb260 in nsAStreamCopier::Start(nsIInputStream*, nsIOutputStream*, nsIEventTarget*, void (*)(void*, nsresult), void*, unsigned int, bool, bool, void (*)(void*, unsigned int)) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/io/nsStreamUtils.cpp:263
13:55:37 INFO - #11 0x7f93ffdfae72 in NS_AsyncCopy(nsIInputStream*, nsIOutputStream*, nsIEventTarget*, nsAsyncCopyMode, unsigned int, void (*)(void*, nsresult), void*, bool, bool, nsISupports**, void (*)(void*, unsigned int)) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/io/nsStreamUtils.cpp:614
13:55:37 INFO - #12 0x7f93ffdfcc30 in NS_CloneInputStream(nsIInputStream*, nsIInputStream**, nsIInputStream**) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/io/nsStreamUtils.cpp:894
13:55:37 INFO - #13 0x7f9403da54d1 in mozilla::dom::InternalResponse::Clone() /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/fetch/InternalResponse.cpp:53
13:55:37 INFO - #14 0x7f9403dabb17 in mozilla::dom::Response::Clone(mozilla::ErrorResult&) const /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/fetch/Response.cpp:203
13:55:37 INFO - #15 0x7f94029dd410 in mozilla::dom::ResponseBinding::clone(JSContext*, JS::Handle<JSObject*>, mozilla::dom::Response*, JSJitMethodCallArgs const&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dom/bindings/./ResponseBinding.cpp:524
13:55:37 INFO - #16 0x7f94039bd7ef in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/bindings/BindingUtils.cpp:2492
13:55:37 INFO - #17 0x7f9407e94fbe in CallJSNative /builds/slave/m-in-l64-asan-0000000000000000/build/src/js/src/jscntxtinlines.h:235
13:55:37 INFO - #18 0x7f9407e94fbe in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) /builds/slave/m-in-l64-asan-0000000000000000/build/src/js/src/vm/Interpreter.cpp:495
13:55:37 INFO - #19 0x7f9407ed28b1 in Interpret(JSContext*, js::RunState&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/js/src/vm/Interpreter.cpp:2597
13:55:37 INFO - #20 0x7f9407eb5971 in js::RunScript(JSContext*, js::RunState&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/js/src/vm/Interpreter.cpp:452
13:55:37 INFO - #21 0x7f9407e9555e in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) /builds/slave/m-in-l64-asan-0000000000000000/build/src/js/src/vm/Interpreter.cpp:521
13:55:37 INFO - #22 0x7f9407e50366 in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) /builds/slave/m-in-l64-asan-0000000000000000/build/src/js/src/vm/Interpreter.cpp:558
13:55:37 INFO - #23 0x7f94089179bf in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/slave/m-in-l64-asan-0000000000000000/build/src/js/src/jsapi.cpp:4338
13:55:37 INFO - #24 0x7f940285820f in mozilla::dom::AnyCallback::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dom/bindings/./PromiseBinding.cpp:78
13:55:37 INFO - #25 0x7f9404b219aa in operator-> /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dom/promise/../../dist/include/mozilla/dom/PromiseBinding.h:128
13:55:37 INFO - #26 0x7f9404b219aa in mozilla::dom::WrapperPromiseCallback::Call(JSContext*, JS::Handle<JS::Value>) /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/promise/PromiseCallback.cpp:208
13:55:37 INFO - #27 0x7f9404b26c6e in mozilla::dom::PromiseCallbackTask::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/promise/Promise.cpp:92
13:55:37 INFO - #28 0x7f9404b0ff86 in mozilla::dom::Promise::PerformMicroTaskCheckpoint() /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/promise/Promise.cpp:435
13:55:37 INFO - #29 0x7f9404a532f8 in mozilla::dom::workers::WorkerPrivate::DoRunLoop(JSContext*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/workers/WorkerPrivate.cpp:4758
13:55:37 INFO - #30 0x7f9404a089a6 in (anonymous namespace)::WorkerThreadPrimaryRunnable::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/workers/RuntimeService.cpp:2693
13:55:37 INFO - #31 0x7f93ffe40514 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThread.cpp:855
13:55:37 INFO - #32 0x7f93ffea227a in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:265
13:55:37 INFO - #33 0x7f94006e4d48 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/glue/MessagePump.cpp:368
13:55:37 INFO - #34 0x7f940068d40c in RunInternal /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:233
13:55:37 INFO - #35 0x7f940068d40c in RunHandler /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:226
13:55:37 INFO - #36 0x7f940068d40c in MessageLoop::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:200
13:55:37 INFO - #37 0x7f93ffe3cf85 in nsThread::ThreadFunc(void*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThread.cpp:356
13:55:37 INFO - #38 0x7f9417b79135 in _pt_root /builds/slave/m-in-l64-asan-0000000000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:212
13:55:37 INFO - #39 0x7f941b094e99 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x7e99)
13:55:37 INFO - Thread T62 (DOM Worker) created by T0 here:
13:55:37 INFO - #0 0x45ea95 in __interceptor_pthread_create /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:175
13:55:37 INFO - #1 0x7f9417b75abd in _PR_CreateThread /builds/slave/m-in-l64-asan-0000000000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:453
13:55:37 INFO - #2 0x7f9417b7563a in PR_CreateThread /builds/slave/m-in-l64-asan-0000000000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:544
13:55:37 INFO - #3 0x7f93ffe3e4bb in nsThread::Init() /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThread.cpp:467
13:55:37 INFO - #4 0x7f9404a7b18a in mozilla::dom::workers::WorkerThread::Create(mozilla::dom::workers::WorkerThreadFriendKey const&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/workers/WorkerThread.cpp:90
13:55:37 INFO - #5 0x7f94049e6546 in mozilla::dom::workers::RuntimeService::ScheduleWorker(JSContext*, mozilla::dom::workers::WorkerPrivate*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/workers/RuntimeService.cpp:1665
13:55:37 INFO - #6 0x7f94049e42a6 in mozilla::dom::workers::RuntimeService::RegisterWorker(JSContext*, mozilla::dom::workers::WorkerPrivate*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/workers/RuntimeService.cpp:1527
13:55:37 INFO - #7 0x7f9404a4f485 in mozilla::dom::workers::WorkerPrivate::Constructor(JSContext*, nsAString_internal const&, bool, mozilla::dom::WorkerType, nsACString_internal const&, mozilla::dom::workers::WorkerLoadInfo*, mozilla::ErrorResult&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/workers/WorkerPrivate.cpp:4333
13:55:37 INFO - #8 0x7f9404a4ee26 in Constructor /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/workers/WorkerPrivate.cpp:4268
13:55:37 INFO - #9 0x7f9404a4ee26 in mozilla::dom::workers::WorkerPrivate::Constructor(mozilla::dom::GlobalObject const&, nsAString_internal const&, mozilla::ErrorResult&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/workers/WorkerPrivate.cpp:4209
13:55:37 INFO - #10 0x7f94030a06eb in mozilla::dom::WorkerBinding::_constructor(JSContext*, unsigned int, JS::Value*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dom/bindings/./WorkerBinding.cpp:721
13:55:37 INFO - #11 0x7f9407ee073e in CallJSNative /builds/slave/m-in-l64-asan-0000000000000000/build/src/js/src/jscntxtinlines.h:235
13:55:37 INFO - #12 0x7f9407ee073e in CallJSNativeConstructor /builds/slave/m-in-l64-asan-0000000000000000/build/src/js/src/jscntxtinlines.h:268
13:55:37 INFO - #13 0x7f9407ee073e in js::InvokeConstructor(JSContext*, JS::CallArgs) /builds/slave/m-in-l64-asan-0000000000000000/build/src/js/src/vm/Interpreter.cpp:596
13:55:37 INFO - #14 0x7f9407ed28a0 in Interpret(JSContext*, js::RunState&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/js/src/vm/Interpreter.cpp:2594
13:55:37 INFO - #15 0x7f9407eb5971 in js::RunScript(JSContext*, js::RunState&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/js/src/vm/Interpreter.cpp:452
13:55:37 INFO - #16 0x7f9407ee1946 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::AbstractFramePtr, JS::Value*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/js/src/vm/Interpreter.cpp:655
13:55:37 INFO - #17 0x7f9407ee1ef7 in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/js/src/vm/Interpreter.cpp:691
13:55:37 INFO - #18 0x7f9408915fe0 in Evaluate(JSContext*, JS::Handle<JSObject*>, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::MutableHandle<JS::Value>) /builds/slave/m-in-l64-asan-0000000000000000/build/src/js/src/jsapi.cpp:4182
13:55:37 INFO - #19 0x7f940891670f in Evaluate /builds/slave/m-in-l64-asan-0000000000000000/build/src/js/src/jsapi.cpp:4209
13:55:37 INFO - #20 0x7f940891670f in JS::Evaluate(JSContext*, JS::AutoObjectVector&, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::MutableHandle<JS::Value>) /builds/slave/m-in-l64-asan-0000000000000000/build/src/js/src/jsapi.cpp:4264
13:55:37 INFO - #21 0x7f94020c71b9 in nsJSUtils::EvaluateString(JSContext*, JS::SourceBufferHolder&, JS::Handle<JSObject*>, JS::CompileOptions&, nsJSUtils::EvaluateOptions const&, JS::MutableHandle<JS::Value>, void**) /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/base/nsJSUtils.cpp:265
13:55:37 INFO - #22 0x7f94020c816b in nsJSUtils::EvaluateString(JSContext*, JS::SourceBufferHolder&, JS::Handle<JSObject*>, JS::CompileOptions&, void**) /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/base/nsJSUtils.cpp:337
13:55:37 INFO - #23 0x7f9402142434 in nsScriptLoader::EvaluateScript(nsScriptLoadRequest*, JS::SourceBufferHolder&, void**) /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/base/nsScriptLoader.cpp:1164
13:55:37 INFO - #24 0x7f940213fb4e in nsScriptLoader::ProcessRequest(nsScriptLoadRequest*, void**) /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/base/nsScriptLoader.cpp:994
13:55:37 INFO - #25 0x7f94021431a6 in nsScriptLoader::ProcessPendingRequests() /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/base/nsScriptLoader.cpp:1192
13:55:37 INFO - #26 0x7f9402167a10 in apply<nsScriptLoader, void (nsScriptLoader::*)()> /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dom/base/../../dist/include/nsThreadUtils.h:574
13:55:37 INFO - #27 0x7f9402167a10 in nsRunnableMethodImpl<void (nsScriptLoader::*)(), true>::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dom/base/../../dist/include/nsThreadUtils.h:666
13:55:37 INFO - #28 0x7f93ffe40514 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThread.cpp:855
13:55:37 INFO - #29 0x7f93ffea227a in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:265
13:55:37 INFO - #30 0x7f94006e3d69 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/glue/MessagePump.cpp:99
13:55:37 INFO - #31 0x7f940068d40c in RunInternal /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:233
13:55:37 INFO - #32 0x7f940068d40c in RunHandler /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:226
13:55:37 INFO - #33 0x7f940068d40c in MessageLoop::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:200
13:55:37 INFO - #34 0x7f9404eb6df7 in nsBaseAppShell::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/widget/nsBaseAppShell.cpp:164
13:55:37 INFO - #35 0x7f9406912348 in nsAppStartup::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/toolkit/components/startup/nsAppStartup.cpp:281
13:55:37 INFO - #36 0x7f9406a0675d in XREMain::XRE_mainRun() /builds/slave/m-in-l64-asan-0000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4202
13:55:37 INFO - #37 0x7f9406a0768d in XREMain::XRE_main(int, char**, nsXREAppData const*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4278
13:55:37 INFO - #38 0x7f9406a0856d in XRE_main /builds/slave/m-in-l64-asan-0000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4498
13:55:37 INFO - #39 0x48a7aa in do_main /builds/slave/m-in-l64-asan-0000000000000000/build/src/browser/app/nsBrowserApp.cpp:294
13:55:37 INFO - #40 0x48a7aa in main /builds/slave/m-in-l64-asan-0000000000000000/build/src/browser/app/nsBrowserApp.cpp:667
13:55:37 INFO - #41 0x7f941a0d176c (/lib/x86_64-linux-gnu/libc.so.6+0x2176c)
13:55:37 INFO - ==2073==ABORTING
13:55:37 INFO - TEST-INFO | Main app process: killed by SIGHUP
13:55:37 INFO - 414 INFO TEST-PASS | dom/workers/test/serviceworkers/test_https_fetch_cloned_response.html | Either active or waiting worker should be available.
13:55:37 INFO - 415 ERROR TEST-UNEXPECTED-FAIL | dom/workers/test/serviceworkers/test_https_fetch_cloned_response.html | application terminated with exit code 1
13:55:37 INFO - runtests.py | Application ran for: 0:03:32.542197
13:55:37 INFO - zombiecheck | Reading PID log: /tmp/tmp32oTp2pidlog
13:55:37 INFO - Stopping web server
13:55:37 INFO - Stopping web socket server
13:55:37 INFO - Stopping ssltunnel
13:55:37 INFO - WARNING | leakcheck | refcount logging is off, so leaks can't be detected!
13:55:37 INFO - runtests.py | Running tests: end.
13:55:37 INFO - SUITE-END | took 223s
13:55:38 ERROR - Return code: 1
13:55:38 ERROR - No tests run or test summary not found
Flags: needinfo?(ehsan)
Flags: needinfo?(bkelly)
Ben, this could be the test failure we were waiting for today!  :-)
Flags: needinfo?(ehsan)
Can't tell what's going on looking from my mobile right now.  I'll investigate.  The layered destructors in ReadStream could incorrectly operate on partially destructed objects, so its a good bet.
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
Flags: needinfo?(bkelly)
Ehsan,

If anything this suggests to me even more that there is some memory corruption in gecko somewhere.  Consider this code:

    if (mRawPtr) {
      NSCAP_RELEASE(this, mRawPtr);
    }

Its crashing because mRawPtr is nullptr in the NSCAP_RELEASE.  But it just passed an mRawPtr if-statement.  The ReadStream does not manually clear its nsCOMPtrs anywhere, so this is not a case of an unsafe off-thread access, either.
Flags: needinfo?(ehsan)
It doesn't have to be mRawPtr that's null.  It could be pointing to something with a null vtable ptr, for instance.
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #5)
> It doesn't have to be mRawPtr that's null.  It could be pointing to
> something with a null vtable ptr, for instance.

Ok, but that suggests to me a garbage pointer.  Again, I don't see how that could happen with this code.
I doubt we are dealing with a garbage pointer here, since this is an ASAN build and trying to dereference the garbage pointer should have crashed earlier.
Flags: needinfo?(ehsan)
(In reply to Treeherder Robot from comment #8)
> log:
> https://treeherder.mozilla.org/logviewer.html#?repo=mozilla-
> inbound&job_id=7685123
> repository: mozilla-inbound
> start_time: 2015-03-16T17:23:44
> who: wkocher[at]mozilla[dot]com
> machine: tst-linux64-spot-778
> buildname: Ubuntu VM 12.04 x64 mozilla-inbound debug test mochitest-4
> revision: b3589228e8b1

This one is a more clear error:

17:30:40     INFO -  Hit MOZ_CRASH(ReadStream not thread-safe) at /builds/slave/m-in-l64-d-0000000000000000000/build/src/dom/cache/ReadStream.cpp:64
17:31:00     INFO -  #01: ReadStreamChild::~ReadStreamChild [dom/cache/ReadStream.cpp:67]
17:31:00     INFO -  #02: mozilla::dom::cache::ReadStream::Release() [dom/cache/ReadStream.cpp:289]
17:31:00     INFO -  #03: nsCOMPtr<nsIInputStream>::operator=(nsIInputStream*) [xpcom/glue/nsCOMPtr.h:547]
17:31:00     INFO -  #04: nsAStreamCopier::Process() [xpcom/io/nsStreamUtils.cpp:342]
17:31:00     INFO -  #05: nsAStreamCopier::Run() [xpcom/io/nsStreamUtils.cpp:420]
17:31:00     INFO -  #06: nsThreadPool::Run() [xpcom/threads/nsThreadPool.cpp:226]
17:31:00     INFO -  #07: nsThread::ProcessNextEvent(bool, bool*) [xpcom/threads/nsThread.cpp:855]
17:31:00     INFO -  #08: NS_ProcessNextEvent(nsIThread*, bool) [xpcom/glue/nsThreadUtils.cpp:265]
17:31:00     INFO -  #09: mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) [ipc/glue/MessagePump.cpp:340]
17:31:00     INFO -  #10: MessageLoop::RunInternal() [ipc/chromium/src/base/message_loop.cc:234]
17:31:00     INFO -  #11: MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:517]
17:31:00     INFO -  #12: nsThread::ThreadFunc(void*) [xpcom/threads/nsThread.cpp:358]
17:31:00     INFO -  #13: _pt_root [nsprpub/pr/src/pthreads/ptthread.c:215]
17:31:00     INFO -  #14: libpthread.so.0 + 0x7e9a
17:31:00     INFO -  #15: libc.so.6 + 0xf42ed

Looks like we are deleting the ReadStreamChild on the wrong thread.  Does this sound like a real issue?
Flags: needinfo?(bkelly)
Yea, if the destructor runs on the wrong then we end up trying AddRef() the object again when we dispatch to the owning thread in NoteClosed().  Doing an AddRef() during destruction is obviously going to do "Bad Things".

Let me see how to fix this.
Flags: needinfo?(bkelly)
I will solve this with delegation to an inner object instead of inheritence.  Cleaner that way all around.
Sounds great!
See Also: → 1144352
I believe this will fix the problem.  Here is a try build:

  https://treeherder.mozilla.org/#/jobs?repo=try&revision=cc1556a121c2

Please see the comment block in StreamControl.h for a full description of the problem and proposed solution.

Andrew, Kyle, I'm flagging for feedback because you rightly had concern about needing a custom Release().  The comment mentioned above has the full story, but here is the gist of the problem.

ReadStream is an nsIInputStream wrapper that sends an IPC message when the stream closes.  This is straightforward in most cases, except for when the stream implicitly closes because its ref-count drops to zero.

Things get ugly in that case because sending the IPC message must be done on a potentially different thread.  Combine this with the fact the actor is not ref-counted and can go away at any time. So I can't just dispatch a message at it.  My solution is essentially to self-ref and perform an explicit Close() when the rec-count drops to 1 (and we haven't already closed, etc).

The ref-cycle also ensures that I can start destruction from the owning thread.  This is really important in order to be able to safely send messages back and forth from the actor.

I'm open to other ideas, though, as I really dislike custom Release().  I just don't see anything cleaner in this case.
Attachment #8579724 - Flags: review?(ehsan)
Attachment #8579724 - Flags: feedback?(khuey)
Attachment #8579724 - Flags: feedback?(continuation)
Note, I probably also could have solved the root problem by just making a strong ref from the actor straight to the ReadStream object without creating the new StreamControl class.  I realized this, however, after I already factored out the StreamControl.

Maybe this is confirmation bias, but I personally like having this separate class.  It means ReadStream does not have to reason about an actor.  It just has to deal with a non-threadsaf ref-counted object.  Likewise, the actor doesn't have to deal with an object running on another thread.  It only has to worry about a ref-counted object on the same thread.

So the division here isolates the thread complexity to ReadStream-to-StreamControl interface and the non-ref-counted complexity to the StreamControl-to-Actor interface.

I guess thats a long winded way of saying I think its cleaner....
Comment on attachment 8579724 [details] [diff] [review]
Teach Cache ReadStream not to AddRef() itself in its destructor. r=ehsan

Dropping review for now while I figure out the widespread try failures.
Attachment #8579724 - Flags: review?(ehsan) → feedback?(ehsan)
Comment on attachment 8579724 [details] [diff] [review]
Teach Cache ReadStream not to AddRef() itself in its destructor. r=ehsan

Review of attachment 8579724 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/cache/CacheStreamControlParent.h
@@ +60,5 @@
>    // is deleted by the PBackground manager.  ActorDestroy() then calls
>    // StreamList::RemoveStreamControl() to clear the weak ref.
>    nsRefPtr<StreamList> mStreamList;
>  
> +  nsTArray<nsRefPtr<StreamControl>> mControlList;

How is access to this protected?  It seems like this array is accessed both on the CacheStreamControlChild and CacheStreamControlParent thread, if I understand things correctly.

::: dom/cache/ReadStream.cpp
@@ +138,2 @@
>    } else {
> +    ref = new ReadStream(control, aReadStream.id(), stream);

Hmm, these two branches seem to be the same...

@@ +236,5 @@
>    if (mClosed) {
>      return;
>    }
>  
> +  mClosed = true;

This is racy, unless I'm mistaken.  You read the value of the atomic first and then update it, so it's possible for two threads to enter NoteClosed(), check mClosed and see that it's false, and then update it to true without knowing about each other, which can cause for example ReadStream::NoteClosedOnOwningThread() to be called twice.

@@ +352,5 @@
> +ReadStream::Release()
> +{
> +  MOZ_ASSERT(int32_t(mRefCnt) > 0, "dup release");
> +  nsrefcnt count = --mRefCnt;
> +  NS_LOG_RELEASE(this, count, "nsPipe");

You meant to say "ReadStream" here.  This probably explains the leaks on try.

@@ +354,5 @@
> +  MOZ_ASSERT(int32_t(mRefCnt) > 0, "dup release");
> +  nsrefcnt count = --mRefCnt;
> +  NS_LOG_RELEASE(this, count, "nsPipe");
> +  if (count == 0) {
> +    delete (this);

Why are you not stabilizing the refcount here?

@@ +361,5 @@
> +  // Automatically close if the only thing keeping us alive is our
> +  // ref-cycle with StreamControl.
> +  if (!mClosed && count == 1) {
> +    Close();
> +    return 1;

Nit: this return statement is unnecessary.

::: dom/cache/StreamControl.h
@@ +49,5 @@
> +//  b) StreamControl holds a weak ref to the IPC actor.  The IPC actor clears
> +//     this in its ActorDestroy().
> +//  c) StreamControl holds a strong ref to ReadStream.
> +//  d) ReadStream holds a strong ref to StreamControl.  This is not a thread
> +//     safe ref, however, so it must be proxy released on the right thread.

I just typed in the same alternative solution you mentioned in comment 15.  I'm not really convinced that this solution is cleaner, as now we need to worry about the lifetime of a third object too.  I find it harder to reason about the lifetimes in this code after this change to be honest...

::: dom/cache/moz.build
@@ +37,5 @@
>      'TypeUtils.h',
>  ]
>  
> +#UNIFIED_SOURCES += [
> +SOURCES += [

Can this be avoided?
Attachment #8579724 - Flags: feedback?(ehsan) → feedback-
Comment on attachment 8579724 [details] [diff] [review]
Teach Cache ReadStream not to AddRef() itself in its destructor. r=ehsan

Kyle told me in IRC that the custom Release() was not crazy given the constraints.
Attachment #8579724 - Flags: feedback?(khuey)
Attachment #8579724 - Flags: feedback?(continuation)
(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #18)
> ::: dom/cache/CacheStreamControlParent.h
> @@ +60,5 @@
> >    // is deleted by the PBackground manager.  ActorDestroy() then calls
> >    // StreamList::RemoveStreamControl() to clear the weak ref.
> >    nsRefPtr<StreamList> mStreamList;
> >  
> > +  nsTArray<nsRefPtr<StreamControl>> mControlList;
> 
> How is access to this protected?  It seems like this array is accessed both
> on the CacheStreamControlChild and CacheStreamControlParent thread, if I
> understand things correctly.

Just to clarify, there are separate ReadStream and StreamControl objects in the child and parent.  The same objects are not shared between the two.  Each actor has their own, separate mControlList.
The implicit Close() that can happen to a stream in its destructor was making things quite difficult.  Talking to Ehsan we came up with a new plan.  This patch now uses outer and inner stream objects.  The outer stream holds the inner stream alive with a strong ref and explicitly calls inner->Close() in ~Outer().  This guarantees the inner stream is always explicitly closed before destruction.

Since this was essentially a rewrite from the last patch, I did not make an interdiff.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=96d440f04d9e
Attachment #8579724 - Attachment is obsolete: true
Comment on attachment 8580431 [details] [diff] [review]
Teach Cache ReadStream not to AddRef() itself in its destructor. r=ehsan

Try is looking pretty good, so far.
Attachment #8580431 - Flags: review?(ehsan)
Comment on attachment 8580431 [details] [diff] [review]
Teach Cache ReadStream not to AddRef() itself in its destructor. r=ehsan

Review of attachment 8580431 [details] [diff] [review]:
-----------------------------------------------------------------

r- for the race condition and the issue about closedCount.

::: dom/cache/CacheStreamControlParent.cpp
@@ +80,5 @@
> +      OptionalFileDescriptorSet::TPFileDescriptorSetParent) {
> +    return;
> +  }
> +
> +  FileDescriptorSetParent* fdSetActor =

Nit: please use auto.

::: dom/cache/ReadStream.cpp
@@ +85,5 @@
> +  void
> +  ForgetOnOwningThread();
> +
> +  // Weak ref to the stream control actor.  The actor will always call either
> +  // CloseStream() or CloseStreamWithoutReporting() before its destroyed.  The

Nit: it's.

@@ +351,5 @@
> +  if (mClosed) {
> +    return;
> +  }
> +
> +  mClosed = true;

This is racy for the reason I explained on the previous patch.  Please use compareExchange().

@@ +366,5 @@
> +  if (mClosed) {
> +    return;
> +  }
> +
> +  mClosed = true;

Ditto.

::: dom/cache/StreamControl.cpp
@@ +54,5 @@
> +      stream->CloseStream();
> +    }
> +  }
> +
> +  MOZ_ASSERT(closedCount > 0);

How does this not assert?  You never increment closedCount

::: dom/cache/moz.build
@@ +37,5 @@
>      'TypeUtils.h',
>  ]
>  
> +#UNIFIED_SOURCES += [
> +SOURCES += [

Please fix this before landing.
Attachment #8580431 - Flags: review?(ehsan) → review-
(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #23)
> @@ +351,5 @@
> > +  if (mClosed) {
> > +    return;
> > +  }
> > +
> > +  mClosed = true;
> 
> This is racy for the reason I explained on the previous patch.  Please use
> compareExchange().

This is restricted to OwningThread only.  I don't think it can race when single threaded.

> ::: dom/cache/StreamControl.cpp
> @@ +54,5 @@
> > +      stream->CloseStream();
> > +    }
> > +  }
> > +
> > +  MOZ_ASSERT(closedCount > 0);
> 
> How does this not assert?  You never increment closedCount

We likely don't have test coverage.  Its only triggered in rare conditions where a Cache is in use, you start using a stream, delete the Cache, and the Cache DOM object is GC'd.

I'll fix it.

> ::: dom/cache/moz.build
> @@ +37,5 @@
> >      'TypeUtils.h',
> >  ]
> >  
> > +#UNIFIED_SOURCES += [
> > +SOURCES += [
> 
> Please fix this before landing.

Yes, I always forget this.
(In reply to Ben Kelly [:bkelly] from comment #24)
> (In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from
> comment #23)
> > @@ +351,5 @@
> > > +  if (mClosed) {
> > > +    return;
> > > +  }
> > > +
> > > +  mClosed = true;
> > 
> > This is racy for the reason I explained on the previous patch.  Please use
> > compareExchange().
> 
> This is restricted to OwningThread only.  I don't think it can race when
> single threaded.

I looked again, and looks like mClosed is indeed only updated from one thread, so this is race-free *now*, but it won't be as soon as someone changes the code in the future in a way that updates mClosed from another thread.  Because I'm sure we will forget to update this at that time, it's best to just use compareExchange() now to be safe.
Attachment #8580729 - Flags: review?(ehsan) → review+
See Also: → 1143579
This was missing a MOZ_OVERRIDE annotation for some macro-implemented AddRef/Release methods. I landed a trivial followup to add this annotation, with the blanket r+ that ehsan granted me for fixes of this sort over on bug 1126447 comment 2:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bffc9502adb1
https://hg.mozilla.org/mozilla-central/rev/edbc24a15142
https://hg.mozilla.org/mozilla-central/rev/bffc9502adb1
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Blocks: 1142803
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.