GVDAH: fix crash on Android caused by invalid assumption of reentrancy on hash_map<>.

GVDAH: fix crash on Android caused by invalid assumption of reentrancy on hash_map<>.

When a container's value's dtor might call another method of the container's,
one must ensure that the dtor is _not_ run in-line as the result of any of the
container's operations.  Specifically here the evil chain is:
GpuVideoEncodeAcceleratorHost::OnNotifyInputDone() -> frame_map_.erase()
VideoFrame::~VideoFrame()
RTCVideoEncoder::Impl::EncodeFrameFinished()
RTCVideoEncoder::Impl::EncodeOneFrame()
GpuVideoEncodeAcceleratorHost::Encode()
GpuVideoEncodeAcceleratorHost::frame_map_.insert()
aaaaaand, boom!

The code at HEAD is invalid for any platform (b/c nothing specs reentrancy for
hash_map) but only bit me on Android (where we use stlport unlike other
platforms).

NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=233238

[sheu, 2013-11-05 20:00:19]

lgtm

Nit: do a find iterator -> erase on iterator to avoid two lookups?

[Ami GONE FROM CHROMIUM, 2013-11-05 20:02:30]

Thanks for the review!


> Nit: do a find iterator -> erase on iterator to avoid two lookups?
>

nah - this is a small enough map that I don't care enough to introduce the
icky syntax.



>
> https://codereview.chromium.org/59863005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2013-11-05 20:06:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fischman@chromium.org/59863005/1

[commit-bot: I haz the power, 2013-11-05 20:43:56]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[Ami GONE FROM CHROMIUM, 2013-11-05 20:46:59]

piman: please OWNERS-review.  (also, you might get a kick out of the story)

[piman, 2013-11-05 21:02:57]

LGTM. Not really surprised... I suspect hash_map isn't the only one in this
case.

[piman, 2013-11-05 21:09:33]

On Tue, Nov 5, 2013 at 1:02 PM, <piman@chromium.org> wrote:

> LGTM. Not really surprised... I suspect hash_map isn't the only one in this
> case.
>

FYI, I quickly checked, and std::vector on gcc 4.6 also would have similar
issues.


> https://codereview.chromium.org/59863005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2013-11-05 21:26:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fischman@chromium.org/59863005/1

[commit-bot: I haz the power, 2013-11-06 01:47:35]

Retried try job too often on linux_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-11-06 02:06:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fischman@chromium.org/59863005/1

[commit-bot: I haz the power, 2013-11-06 06:18:50]

Retried try job too often on linux_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-11-06 06:41:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fischman@chromium.org/59863005/1

[commit-bot: I haz the power, 2013-11-06 06:45:29]

Change committed as 233238