746810 - ASSERTION: Target channel not in the process of opening: 'ChannelOpening == aTargetChan->mChannelState

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Oleg Romashin (:romaxa)]

Attachment: Fix proposal

On attempt to initialize Compositor Parent channel for OMTC rendering in embedding setup: see bug 746800#c1
I see next assertion:
**************************************
1034701888[3dc647f0]: ###!!! ASSERTION: Target channel not in the process of opening: 'ChannelOpening == aTargetChan->mChannelState', file ipc/glue/AsyncChannel.cpp, line 368
###!!! ASSERTION: Target channel not in the process of opening: 'ChannelOpening == aTargetChan->mChannelState', file ipc/glue/AsyncChannel.cpp, line 368
1034701888[3dc647f0]: ###!!! ASSERTION: Target channel not in the process of opening: 'ChannelOpening == aTargetChan->mChannelState', file ipc/glue/AsyncChannel.cpp, line 375
###!!! ASSERTION: Target channel not in the process of opening: 'ChannelOpening == aTargetChan->mChannelState', file ipc/glue/AsyncChannel.cpp, line 375
**********************************
which is coming right after PostTask(NewRunnableMethod(aTargetChan, &AsyncChannel::OnOpenAsSlave,)
http://mxr.mozilla.org/mozilla-central/source/ipc/glue/AsyncChannel.cpp#342

it looks like AsyncChannel::OnOpenAsSlave executed in child thread 
before mChannelState = ChannelOpening set

Moving mChannelState = ChannelOpening before PostTask call, fixing the problem

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 616364 [details] [diff] [review]
Fix proposal

This makes sense.  I was able to reproduce the assertion when running the ipdl/c++ tests, |make -C $objdir/ipc/ipdl/test/cxx check|.  Please test your patch against those tests and all good.

Thanks!

Comment 2

[Oleg Romashin (:romaxa)]

Comment on attachment 616364 [details] [diff] [review]
Fix proposal

Tested this with make check
Cannot catch that locally on desktop gtk debug build (probably different timings)
Test same with patch and don't see any new errors.

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

This code runs in native-fennec, and fixes a race condition/logic error that native-fennec is vulnerable to hit.  If there are debug builds of native-fennec it's quite likely they were tripping this assertion.

The fix itself has basically no risk.  I'm not sure if the defect it fixes could cause user-visible issues, but it's possible.

Comment 4

[Oleg Romashin (:romaxa)]

https://tbpl.mozilla.org/?tree=Try&rev=55d3402d5186

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8ef46e071f65

Any chance of getting a test for this?

Comment 6

[Oleg Romashin (:romaxa)]

not sure, this is timing dependent issue.. depends on how fast thread could be created

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/8ef46e071f65

Comment 8

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

This is caught by existing tests.  They don't run on tinderbox though.