Fix a crash by asynchronizing destruction.

Fix a crash by asynchronizing destruction.

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

[MAD, 2013-09-13 19:14:19]

How about this change to prevent the use after free caught by SyzyASAN.

Thanks!

BYE
MAD

[sky, 2013-09-13 19:33:58]

I have a feeling this effects other places too. Is it possible to have
HWNDMessageHandler::OnMouseRange check for destruction?

[MAD, 2013-09-13 19:59:48]

Ho! I just had an idea... What if we keep a weak ptr in the OnMouseRange
method and chec it before calling SetMsgHandled?

And even if we would fix the HWNDMessageHandler, there might be other
pieced of code in the stack like the link view, the base class view, the
event, etc... that could affected by the destruction of the window... So I
still think this fix is needed... Unless there is already an understanding
in all those classes that once they made that call, they could be destroyed
on the return of the call. What do you think?


On Fri, Sep 13, 2013 at 3:33 PM, <sky@chromium.org> wrote:

> I have a feeling this effects other places too. Is it possible to have
> HWNDMessageHandler::**OnMouseRange check for destruction?
>
>
https://codereview.chromium.**org/23441062/<https://codereview.chromium.org/2...
>

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

[sky, 2013-09-13 20:50:23]

On Fri, Sep 13, 2013 at 12:59 PM, Marc-Andre Decoste <mad@chromium.org> wrote:
> Ho! I just had an idea... What if we keep a weak ptr in the OnMouseRange
> method and chec it before calling SetMsgHandled?

Yes, go for it.


> And even if we would fix the HWNDMessageHandler, there might be other pieced
> of code in the stack like the link view, the base class view, the event,
> etc... that could affected by the destruction of the window... So I still
> think this fix is needed... Unless there is already an understanding in all
> those classes that once they made that call, they could be destroyed on the
> return of the call. What do you think?

It's certainly true moving the fix to HWNDMEssageHandler may shift the
crash, but I don't think this crash is that uncommon. There are
certainly places like this we have made resilient to deletion while
dispatching.

  -Scott

>
>
> On Fri, Sep 13, 2013 at 3:33 PM, <sky@chromium.org> wrote:
>>
>> I have a feeling this effects other places too. Is it possible to have
>> HWNDMessageHandler::OnMouseRange check for destruction?
>>
>> https://codereview.chromium.org/23441062/
>
>

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

[markusheintz_, 2013-09-16 16:40:12]

Thanks a lot for deep diving into this and fixing the issue.

LGTM

[commit-bot: I haz the power, 2013-09-17 12:20:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mad@chromium.org/23441062/4001

[commit-bot: I haz the power, 2013-09-17 15:00:25]

Change committed as 223616