danakj@ could you please look into this.please feel free to re-assigned back if needed. thanks in advance !

Crashes in Callback mean someone is running a null callback or has deleted something bound in a callback usually.

In this case it looks like PersistentBase is gone but the bound callback is trying to destroy it? Passing to haraken who will probably know someone best suited to look at what looks like Oilpan/Mojo interactions.

Issue 647705 has been merged into this issue.

Issue 645102 has been merged into this issue.

As I commented in https://codereview.chromium.org/2400903002/#msg8, I suspect there will be no easy work-around on this issue.

The only way to solve this problem is to leak MessageLoop or remove the shutdown sequence. I'm actively working on it in https://codereview.chromium.org/2309153002, but it will take a bit more time to land it.

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas

Re comment #5: Is there really no other way to address this? For example, wouldn't it be possible for things which own bindings to close their bindings on Blink shutdown? We don't have that many Blink objects owning bindings right now, do we? Sorry if I'm misunderstanding the issue.

Also is this essentially a dupe of  bug 647127 ?

The objects that own InterfacePtrs are supposed to close their bindings in their finalizer. To prevent dispatch to them during lazy sweeping (before the finalizer is called) Persistent/WeakPersistent pointers are used. This is failing in this case because the weakPersistent pointer is not being invalidated AND the finalizer is not being run and yet the object has been destroyed.

To me the solutions are either:

1) Fix WeakPersistent so that it prevents dispatch in this case.
2) Execute the finalizers on Blink shutdown.

There's an additional detail that haraken@ raised in https://codereview.chromium.org/2400903002/ which is that in this case we're crashing because the memory backing the WeakPersistent handle has been freed. I would argue that this is a bug in how WeakPersistent is implemented but I am not familiar with the Oilpan internals mandating this design.

 Issue 654584  has been merged into this issue.

 Issue 647127  has been merged into this issue.

The underlying problem is that Blink is assuming that content/ doesn't touch Blink things after calling blink::shutdown(). We must clear all pointers from content/ to Blink before content/ calls blink::shutdown(). If we follow the lifetime model, we need to destruct all WeakPersistents before blink::shutdown() so that ~MessageLoop doesn't touch any Blink things.

We have so far tried to follow the lifetime model, but, as this crash indicates, it's not always easy to follow the model. That's why I think a right fix will be to remove the shutdown sequence.

Maybe we can land some work-around (e.g., not freeing the backing storage of WeakPersistents/Persistents), but it has a risk of causing other use-after-frees.

ClusterFuzz testcase is verified as fixed, closing issue.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

This is the #2 crash for mac renderer in latest stable - 54.0.2840.59, think we need to merge the fix to M54.

haraken@ can you please confirm, tagging accordingly.

I apologize I couldn't spend much time on fixing this issue. I'll work on this at the first priority on Monday.

I think that one possible quick fix is to stop calling freePersistentNode from ~PersistentBase after blink::shutdown is called.

Uploaded a fix: https://codereview.chromium.org/2423743003

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/10d68110da927986332bd753b17c08db0feac1c8

commit 10d68110da927986332bd753b17c08db0feac1c8
Author: haraken <haraken@chromium.org>
Date: Mon Oct 17 05:24:22 2016

Stop calling Persistent::uninitialize after blink::shutdown is called.

This CL adds a short-term hack to prevent use-after-frees during a shutdown sequence.
Currently the following scenario can happen:

1) blink::shutdown() frees the underlying storage for persistent nodes.
2) ~MessageLoop() destructs some Chromium-side objects that hold Persistent.
It touches the underlying storage and crashes.

In middle term, we should entirely remove the shutdown sequence and get rid of the hack.

BUG= 653688 

Review-Url: https://codereview.chromium.org/2423743003
Cr-Commit-Position: refs/heads/master@{#425620}

[modify] https://crrev.com/10d68110da927986332bd753b17c08db0feac1c8/third_party/WebKit/Source/platform/heap/Persistent.h

haraken@ can we please get the CL merge to M55.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/76db5a37e59526f1e3592c678a99b99cd9ae5569

commit 76db5a37e59526f1e3592c678a99b99cd9ae5569
Author: Kentaro Hara <haraken@chromium.org>
Date: Thu Oct 20 17:22:09 2016

Stop calling Persistent::uninitialize after blink::shutdown is called.

This CL adds a short-term hack to prevent use-after-frees during a shutdown sequence.
Currently the following scenario can happen:

1) blink::shutdown() frees the underlying storage for persistent nodes.
2) ~MessageLoop() destructs some Chromium-side objects that hold Persistent.
It touches the underlying storage and crashes.

In middle term, we should entirely remove the shutdown sequence and get rid of the hack.

BUG= 653688 

Review-Url: https://codereview.chromium.org/2423743003
Cr-Commit-Position: refs/heads/master@{#425620}
(cherry picked from commit 10d68110da927986332bd753b17c08db0feac1c8)

Review URL: https://codereview.chromium.org/2430553002 .

Cr-Commit-Position: refs/branch-heads/2883@{#213}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/76db5a37e59526f1e3592c678a99b99cd9ae5569/third_party/WebKit/Source/platform/heap/Persistent.h

The crash with magic signature mojo::InterfacePtr<blink::mojom::blink::PermissionService>::reset is not seen anymore after the build 55.0.2869.0

Link to the builds:
https://crash.corp.google.com/browse?q=custom_data.ChromeCrashProto.magic_signature_1.name%3D%27mojo%3A%3AInterfacePtr%3Cblink%3A%3Amojom%3A%3Ablink%3A%3APermissionService%3E%3A%3Areset%27&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=#samplereports:5,productversion:1000

Currently this is a top # 1 renderer crasher on Mac stable 54.0.2840.71 with statistics as 63.75% with 38584 instances of crashes from 32600 unique client Ids.
https://crash.corp.google.com/browse?q=product.name%3D%27Chrome_Mac%27%20AND%20product.version%3D%2754.0.2840.71%27%20AND%20custom_data.ChromeCrashProto.ptype%3D%27renderer%27%20AND%20custom_data.ChromeCrashProto.magic_signature_1.name%3D%27mojo%3A%3AInterfacePtr%3Cblink%3A%3Amojom%3A%3Ablink%3A%3APermissionService%3E%3A%3Areset%27&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D

haraken@, does it make sense to merge this to M-54 as well?

Yes. I'll request a merge to M54.

[Automated comment] Request affecting a post-stable build (M54), manual review required.

This is the #1 crash in current mac stable- 54.0.2840.71 ,64.38%, 49102 reports from 40059 unique clients.

We need to get this to M54 before next refresh. Richard, could you please approve the merge.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/76db5a37e59526f1e3592c678a99b99cd9ae5569

commit 76db5a37e59526f1e3592c678a99b99cd9ae5569
Author: Kentaro Hara <haraken@chromium.org>
Date: Thu Oct 20 17:22:09 2016

Stop calling Persistent::uninitialize after blink::shutdown is called.

This CL adds a short-term hack to prevent use-after-frees during a shutdown sequence.
Currently the following scenario can happen:

1) blink::shutdown() frees the underlying storage for persistent nodes.
2) ~MessageLoop() destructs some Chromium-side objects that hold Persistent.
It touches the underlying storage and crashes.

In middle term, we should entirely remove the shutdown sequence and get rid of the hack.

BUG= 653688 

Review-Url: https://codereview.chromium.org/2423743003
Cr-Commit-Position: refs/heads/master@{#425620}
(cherry picked from commit 10d68110da927986332bd753b17c08db0feac1c8)

Review URL: https://codereview.chromium.org/2430553002 .

Cr-Commit-Position: refs/branch-heads/2883@{#213}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/76db5a37e59526f1e3592c678a99b99cd9ae5569/third_party/WebKit/Source/platform/heap/Persistent.h

lgtm, Approved for merging into M54

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/21412c271cc49cd409b96b94013d1ce054159202

commit 21412c271cc49cd409b96b94013d1ce054159202
Author: Kentaro Hara <haraken@chromium.org>
Date: Fri Oct 28 08:51:31 2016

Stop calling Persistent::uninitialize after blink::shutdown is called.

This CL adds a short-term hack to prevent use-after-frees during a shutdown sequence.
Currently the following scenario can happen:

1) blink::shutdown() frees the underlying storage for persistent nodes.
2) ~MessageLoop() destructs some Chromium-side objects that hold Persistent.
It touches the underlying storage and crashes.

In middle term, we should entirely remove the shutdown sequence and get rid of the hack.

BUG= 653688 

Review-Url: https://codereview.chromium.org/2423743003
Cr-Commit-Position: refs/heads/master@{#425620}
(cherry picked from commit 10d68110da927986332bd753b17c08db0feac1c8)

Review URL: https://codereview.chromium.org/2456293002 .

Cr-Commit-Position: refs/branch-heads/2840@{#792}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/21412c271cc49cd409b96b94013d1ce054159202/third_party/WebKit/Source/platform/heap/Persistent.h

This crash is still seen in latest stable version 54.0.2840.87 with 325 instances from 308 unique client ids on mac

Link to the builds:
https://crash.corp.google.com/browse?q=custom_data.ChromeCrashProto.magic_signature_1.name%3D%27mojo%3A%3AInterfacePtr%3Cblink%3A%3Amojom%3A%3Ablink%3A%3APermissionService%3E%3A%3Areset%27%20AND%20product.name%3D%27Chrome_Mac%27&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D#samplereports:5,productversion:1000

haraken@, Could you please look into this as still crashes are seen post fix in comment #28

Please see issue 650637. We never merged the fix into M54. Merge has now been requested on that bug.

Just to update:

This is the top#1 renderer crash on mac in stable version 54.0.2840.87 with 4346 instances from 3551 unique client Ids

Could you please merge this to M54 ASAP as the instances are increasing.

haraken@'s change was already been merged to M-54 and should have been included in 54.0.2840.87.

As was already explained in comment #32, the remaining crash reports in M54 are actually tracked by issue 650637, and the fix was merged into M54 recently. No 54 refresh has been done yet since the merge.

This crash is not seen anymore on mac latest stable version 54.0.2840.98

Removing EditIssue view restrictions from ClusterFuzz filed bugs. If you believe that this issue should still be restricted, please reapply the label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot