Closed Bug 1501793 Opened Last year Closed Last year

Assertion failure: !mRawPtr in APZUpdater::~APZUpdater due to APZUpdater::RunOnUpdaterThread

Categories

(Core :: Graphics: WebRender, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: aosmond, Assigned: aosmond)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

I tripped the following assert/backtrace when shutting down a debug build on Linux:

Assertion failure: !mRawPtr

(gdb) bt
#0  0x00007fd1375359d0 in __GI___nanosleep (requested_time=requested_time@entry=0x7fd116b3efb0, remaining=remaining@entry=0x7fd116b3efb0)
    at ../sysdeps/unix/sysv/linux/nanosleep.c:28
#1  0x00007fd1375358aa in __sleep (seconds=0) at ../sysdeps/posix/sleep.c:55
#2  0x00007fd12bf2a5a2 in ah_crap_handler(int) (signum=11) at /moz/gecko/toolkit/xre/nsSigHandlers.cpp:102
#3  0x00007fd12bf1200f in nsProfileLock::FatalSignalHandler(int, siginfo_t*, void*) (signo=11, info=0x7fd116b3f1f0, context=0x7fd116b3f0c0)
    at /moz/gecko/toolkit/profile/nsProfileLock.cpp:187
#4  0x00007fd138397890 in <signal handler called> () at /lib/x86_64-linux-gnu/libpthread.so.0
#5  0x00007fd127c166f3 in already_AddRefed<mozilla::Runnable>::~already_AddRefed() (this=<optimized out>)
    at /moz/gecko-dev/obj-clang-debug-x86_64-pc-linux-gnu/dist/include/mozilla/AlreadyAddRefed.h:126
#6  0x00007fd128cc9bd2 in RunnableFunction<void (*)(already_AddRefed<mozilla::Runnable>), mozilla::Tuple<already_AddRefed<mozilla::Runnable> > >::~RunnableFunction() (this=0x7fd104074f00) at /moz/gecko/ipc/chromium/src/base/task.h:345
#7  0x00007fd128cc9bee in RunnableFunction<void (*)(already_AddRefed<mozilla::Runnable>), mozilla::Tuple<already_AddRefed<mozilla::Runnable> > >::~RunnableFunction() (this=0x7fd104074f00) at /moz/gecko/ipc/chromium/src/base/task.h:344
#8  0x00007fd127d2bbdb in mozilla::Runnable::Release() (this=0x7fd104074f00) at /moz/gecko/xpcom/threads/nsThreadUtils.cpp:50
#9  0x00007fd128cc6000 in std::_Destroy_aux<false>::__destroy<mozilla::layers::APZUpdater::QueuedTask*>(mozilla::layers::APZUpdater::QueuedTask*, mozilla::layers::APZUpdater::QueuedTask*) (__first=0x7fd103fd0c00, __last=0x7fd103fd0e00)
    at /usr/bin/../lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/c++/7.3.0/bits/stl_construct.h:108
#10 0x00007fd128cc5f06 in std::deque<mozilla::layers::APZUpdater::QueuedTask, std::allocator<mozilla::layers::APZUpdater::QueuedTask> >::_M_destroy_data_aux(std::_Deque_iterator<mozilla::layers::APZUpdater::QueuedTask, mozilla::layers::APZUpdater::QueuedTask&, mozilla::layers::APZUpdater::QueuedTask*>, std::_Deque_iterator<mozilla::layers::APZUpdater::QueuedTask, mozilla::layers::APZUpdater::QueuedTask&, mozilla::layers::APZUpdater::QueuedTask*>) (this=0x7fd1372c7328, __first={mLayersId = {mId = 4294967298}, mRunnable = {mRawPtr = 0x7fd103ff6540}}, __last=
    {mLayersId = {mId = 16493559407081481444}, mRunnable = {mRawPtr = 0xe4e4e4e4e4e4e4e4}})
    at /usr/bin/../lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/c++/7.3.0/bits/deque.tcc:850
#11 0x00007fd128cc5e5e in std::deque<mozilla::layers::APZUpdater::QueuedTask, std::allocator<mozilla::layers::APZUpdater::QueuedTask> >::_M_destroy_data(std::_Deque_iterator<mozilla::layers::APZUpdater::QueuedTask, mozilla::layers::APZUpdater::QueuedTask&, mozilla::layers::APZUpdater::QueuedTask*>, std::_Deque_iterator<mozilla::layers::APZUpdater::QueuedTask, mozilla::layers::APZUpdater::QueuedTask&, mozilla::layers::APZUpdater::QueuedTask*>, std::allocator<mozilla::layers::APZUpdater::QueuedTask> const&) (this=0x7fd13783d680 <_IO_2_1_stderr_>, __first=..., __last=...)
    at /usr/bin/../lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/c++/7.3.0/bits/stl_deque.h:2072
#12 0x00007fd128ca3ed7 in std::deque<mozilla::layers::APZUpdater::QueuedTask, std::allocator<mozilla::layers::APZUpdater::QueuedTask> >::~deque() (this=0x7fd1372c7328) at /usr/bin/../lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/c++/7.3.0/bits/stl_deque.h:1045
#13 0x00007fd128c8b868 in mozilla::layers::APZUpdater::~APZUpdater() (this=0x7fd1372c71f0) at /moz/gecko/gfx/layers/apz/src/APZUpdater.cpp:47
#14 0x00007fd128c8b91e in mozilla::layers::APZUpdater::~APZUpdater() (this=0x7fd1372c71f0) at /moz/gecko/gfx/layers/apz/src/APZUpdater.cpp:38
#15 0x00007fd128c795fc in mozilla::layers::APZUpdater::Release() (this=0x7fd1372c71f0)
    at /moz/gecko-dev/obj-clang-debug-x86_64-pc-linux-gnu/dist/include/mozilla/layers/APZUpdater.h:44
#16 0x00007fd128d4ad7c in mozilla::layers::APZCTreeManagerParent::~APZCTreeManagerParent() (this=0x7fd10f383680)
    at /moz/gecko/gfx/layers/ipc/APZCTreeManagerParent.cpp:30

It looks like RunnableFunction stores the underlying runnable as an already_AddRefed<mozilla::Runnable>. We may end up storing that inside a QueueTask in APZUpdater::mUpdaterQueue in APZUpdater::RunOnUpdaterThread. 

I think if we change APZUpdater::RunOnControllerThread to convert the already_AddRefed<Runnable> aTask parameter into a RefPtr<Runnable> which we move into the NewRunnableMethod, instead of moving the already_AddRefed directly, we will avoid the assert.
The practical outcome on non-debug builds is a small memory leak, although I'm not familiar with the life cycle of a PAPZCTreeManagerParent to know how frequently it happens.
When APZUpdater::mUpdaterQueue still contains entries, and it is
destroyed, we may trip an assert due to destroyed an already_AddRefed
object without taking its contents into a RefPtr. This is because
APZUpdater::RunOnControllerThread would pass an already_AddRefed object
directly into NewRunnableFunction as the object, instead of a RefPtr
object. This caused templated object to store an already_AddRefed object
as a result, and when we wanted to drop the object, it complained.
Storing it as a RefPtr should cause everything to be freed normally.
Assignee: nobody → aosmond
Priority: -- → P2
This builds / works for me locally, but let's just make sure it works on try, because I don't trust the chain of implicit constructors required:

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3d02bb7e0b3ab605db22496ba7174859af337e70
Pushed by aosmond@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b304191da6b7
Fix a shutdown assert in APZUpdater::~APZUpdater. r=kats
https://hg.mozilla.org/mozilla-central/rev/b304191da6b7
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
See Also: → 1502495
You need to log in before you can comment on or make changes to this bug.