New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 654695 link

Starred by 61 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug

Blocking:
issue 669773

Restricted
  • Only users with EditIssue permission may comment.


Show other hotlists

Hotlists containing this issue:
Hotlist-1


Sign in to add a comment

Using System IME Crashed on macOS Sierra

Reported by miechalz...@gmail.com, Oct 11 2016

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/53.0.2785.143 Safari/537.36

Steps to reproduce the problem:
1. Login im.dingtalk.com
2. Start Chat With Friend by using System IME
3. No signs before Crashed

What is the expected behavior?
Input Method For Normal

What went wrong?
No signs before Crashed

Crashed report ID: 7e8aefa2-1137-42b0-bf1d-d09e0de79983.dmp
c5f4f9cb-2d00-4147-9df3-42862e6dd5ea.dmp
ef934498-6816-4794-bb20-77596582b371.dmp

How much crashed? Entity Chrome App

Is it a problem with a plugin? N/A 

Did this work before? No 

Chrome version: 53.0.2785.143  Channel: stable
OS Version: OS X 10.12.0
Flash Version: Shockwave Flash 23.0 r0

Just Crash On macOS Sierra By Using System IME
 
IME-Chrome-Crash-Dump.zip
187 KB Download
Showing comments 32 - 131 of 131 Older
**** Bulk edit -  please ignore if not applicable ****

A friendly reminder that M55 Stable is launch is coming soon! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch ASAP so it gets enough baking time in Beta (before Stable promotion). Thank you!

Also due to Thanksgiving holidays in US, please make sure all fixes are ready and merged to M55 latest by 5:00 PM PT Friday, 11/18/16.
Hello pichu.baby@,

Can you see if you still get your crash using the latest Canary (56.0.2911.0 or later)?

Project Member

Comment 34 by bugdroid1@chromium.org, Nov 7 2016

Labels: -merge-approved-55 merge-merged-2883
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/da364879b2e82d42525cb8942c652c2403d43ffb

commit da364879b2e82d42525cb8942c652c2403d43ffb
Author: erikchen <erikchen@chromium.org>
Date: Mon Nov 07 18:15:35 2016

[Merge to 2883] Fix memory leaks in macOS Sierra for IME.

> The documentation for NSTextInputClient states that:
>   -selectedRange should return { NSNotFound, 0 } if there is no selection range.
>   -attributedSubstringForProposedRange:actualRange: must be able to handle
>   invalid ranges.
>
> The previous implementations in RenderWidgetHostViewCocoa failed to do either,
> which results in catastrophic memory leaks on macOS Sierra during IME. In an
> ASAN build, AppKit attempts to allocate ~2^64 bytes of memory.
>
> BUG= 654695 
>
> Review-Url: https://codereview.chromium.org/2480893002
> Cr-Commit-Position: refs/heads/master@{#430116}
> (cherry picked from commit 7d85f23cb0235db06b0b6c2de1dc29ae5eaeb8f5)

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

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

[modify] https://crrev.com/da364879b2e82d42525cb8942c652c2403d43ffb/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/da364879b2e82d42525cb8942c652c2403d43ffb/content/browser/renderer_host/render_widget_host_view_mac_unittest.mm

Status: Fixed (was: Assigned)
Hi erikchen@ - just confirming that you checked the Canary to make sure the issues you were seeing were actually fixed by your change?

I confirmed that the issue I was seeing under ASAN was fixed. I could not reproduce the issue without ASAN.
Hello shrike,

I have tested on 56.0.2911.0 and 56.0.2914.0, and it doesn't crash.

I got the binary from this site:
https://chromium.woolyss.com/download/#mac

Thanks for your help : )

    Pichu

Comment 39 by ajha@chromium.org, Nov 9 2016

Labels: TE-Verified-55.0.2883.44 TE-Verified-M55
Verified the fix on the latest M-55(55.0.2883.44) on Mac OS 10.12.1 as per C#31 and this is working as intended and no crash is seen using Pinyan traditional keyboard layout.

Note: I was able to reproduce the crash on the build without the fix on chrome version: 55.0.2883.35.
M55 doesn't go to stable for a month. Should we consider merging this back to M54 stable and refreshing?
Labels: Merge-Request-54
At the least we should merge to M-54 so that it gets caught in any refreshes. I suggest we add Merge-Request-54, but also let it bake a while longer?

Comment 42 by dimu@chromium.org, Nov 9 2016

Labels: -Merge-Request-54 Merge-Review-54 Hotlist-Merge-Review
[Automated comment] Request affecting a post-stable build (M54), manual review required.
Status: Assigned (was: Fixed)
My fix introduces a different IME issue: https://bugs.chromium.org/p/chromium/issues/detail?id=664554

Theoretically, removing the conditional should fix the issue:
"""
 3020 - (NSRange)selectedRange {
 3021   if (... selectedRange_.length == 0)
 3022     return NSMakeRange(NSNotFound, 0);
"""

However, doing so reintroduces the memory leak on Sierra. Worse, even without this change, the memory leak can still be triggered on macOS Sierra by having a non-zero selected range and then triggering IME. Did some live debugging, looks like the range passed to:

"""
frame #10: 0x00007fff9c08c812 HIToolbox`-[IMKInputSession _coreAttributesFromRange:whichAttributes:completionHandler:] + 61
frame #11: 0x00007fff9c08e890 HIToolbox`-[IMKInputSession attributedSubstringFromRange:completionHandler:] + 186
"""

really does cause a malloc(2^64 - 1 - selectedRange.location). Yikes! Unfortunately, the logic for popping up the IME box is cross-process (XPC) and non-trivially complex, so I don't know exactly what's going wrong. I'm going to try to repro this issue in a standalone app.
Project Member

Comment 44 by bugdroid1@chromium.org, Nov 12 2016

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

commit f31b9cbfe43d85c9b631c1da8a01997b9ef24e9c
Author: erikchen <erikchen@chromium.org>
Date: Sat Nov 12 00:47:34 2016

Revert of Fix RenderWidgetHostViewMac compliance with NSTextInputClient. (patchset #2 id:20001 of https://codereview.chromium.org/2480893002/ )

Reason for revert:
Breaks some IME [hold down a vowel, English input language] without fully fixing IME issues on macOS Sierra.

https://bugs.chromium.org/p/chromium/issues/detail?id=664554#c5

Original issue's description:
> Fix memory leaks in macOS Sierra for IME.
>
> The documentation for NSTextInputClient states that:
>   -selectedRange should return { NSNotFound, 0 } if there is no selection range.
>   -attributedSubstringForProposedRange:actualRange: must be able to handle
>   invalid ranges.
>
> The previous implementations in RenderWidgetHostViewCocoa failed to do either,
> which results in catastrophic memory leaks on macOS Sierra during IME. In an
> ASAN build, AppKit attempts to allocate ~2^64 bytes of memory.
>
> BUG= 654695 
>
> Committed: https://crrev.com/7d85f23cb0235db06b0b6c2de1dc29ae5eaeb8f5
> Cr-Commit-Position: refs/heads/master@{#430116}

TBR=avi@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 654695 

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

[modify] https://crrev.com/f31b9cbfe43d85c9b631c1da8a01997b9ef24e9c/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/f31b9cbfe43d85c9b631c1da8a01997b9ef24e9c/content/browser/renderer_host/render_widget_host_view_mac_unittest.mm

I reproduced the bug in a test application. Filed radar: 29229715. 
debugger_output.rtf
7.4 KB Download
IME_test.zip
34.5 KB Download
Project Member

Comment 46 by bugdroid1@chromium.org, Nov 12 2016

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

commit 375a0e0246639ae6f9ed2f6561d657187f9431c1
Author: erikchen <erikchen@chromium.org>
Date: Sat Nov 12 00:58:53 2016

[Merge to 2883] Revert of Fix RenderWidgetHostViewMac compliance with NSTextInputClient.

> Reason for revert:
> Breaks some IME [hold down a vowel, English input language] without fully fixing IME issues on macOS Sierra.
>
> https://bugs.chromium.org/p/chromium/issues/detail?id=664554#c5
>
> Original issue's description:
> > Fix memory leaks in macOS Sierra for IME.
> >
> > The documentation for NSTextInputClient states that:
> >   -selectedRange should return { NSNotFound, 0 } if there is no selection range.
> >   -attributedSubstringForProposedRange:actualRange: must be able to handle
> >   invalid ranges.
> >
> > The previous implementations in RenderWidgetHostViewCocoa failed to do either,
> > which results in catastrophic memory leaks on macOS Sierra during IME. In an
> > ASAN build, AppKit attempts to allocate ~2^64 bytes of memory.
> >
> > BUG= 654695 
> >
> > Committed: https://crrev.com/7d85f23cb0235db06b0b6c2de1dc29ae5eaeb8f5
> > Cr-Commit-Position: refs/heads/master@{#430116}
>
> TBR=avi@chromium.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG= 654695 
>
> Review-Url: https://codereview.chromium.org/2497073002
> Cr-Commit-Position: refs/heads/master@{#431713}
> (cherry picked from commit f31b9cbfe43d85c9b631c1da8a01997b9ef24e9c)

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

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

[modify] https://crrev.com/375a0e0246639ae6f9ed2f6561d657187f9431c1/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/375a0e0246639ae6f9ed2f6561d657187f9431c1/content/browser/renderer_host/render_widget_host_view_mac_unittest.mm

**** Bulk edit -  please ignore if not applicable ****


A friendly reminder that M55 Stable is launch is coming soon! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch ASAP so it gets enough baking time in Beta (before Stable promotion). Thank you!

Also due to Thanksgiving holidays in US, please make sure fix is ready and merged to M55 latest by 5:00 PM PT Friday, 11/18/16 (sooner the better).
What are the next steps? Are we blocked waiting for traction on the radar? Should we escalate this to apple?
I need to reland my CL with english IME fixed, and see if users still have issues.
Project Member

Comment 50 by bugdroid1@chromium.org, Nov 14 2016

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

commit 1409990053e62bfbd6744d7f88524e0c0ee95797
Author: erikchen <erikchen@chromium.org>
Date: Mon Nov 14 23:05:17 2016

Fix incorrect implementation of NSTextInputClient.

The documentation for NSTextInputClient states that:
  -selectedRange should return { NSNotFound, 0 } if there is no selection range.
  -attributedSubstringForProposedRange:actualRange: must be able to handle
  invalid ranges.

The previous implementations in RenderWidgetHostViewCocoa failed to do either.

BUG= 654695 

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

[modify] https://crrev.com/1409990053e62bfbd6744d7f88524e0c0ee95797/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/1409990053e62bfbd6744d7f88524e0c0ee95797/content/browser/renderer_host/render_widget_host_view_mac_unittest.mm

pichu.baby@gmail.com: Can you test tomorrow's Canary and see if the problem is still around? [Trying to figure out whether the macOS bug I filed is the root cause, or whether there's another Chrome issue].
pichu.baby@ Could you please respond for the comment #51?
Cc: tapted@chromium.org karandeepb@chromium.org erikc...@chromium.org
 Issue 666195  has been merged into this issue.
Hi Erik. The issue is still ongoing right? I think the crash under ASAN on switching to Pinyin is still reproducible.
**** Bulk edit -  please ignore if not applicable ****

A friendly reminder that M55 Stable is launch is coming soon! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch latest by November 25th, 5:00 PM PST in order to make into the desktop Stable final build cut. Thank you!
Labels: -M-55 M-56
There's nothing more to be done for M-55.
Awesome,thanks so much! I will confirm it.
Status: ExternalDependency (was: Assigned)
Blocking: 669773
Project Member

Comment 60 by sheriffbot@chromium.org, Dec 12 2016

Labels: FoundIn-M-57 Fracas
Users experienced this crash on the following builds:

Mac Dev 57.0.2946.0 -  2.00 CPM, 10 reports, 8 clients (signature [Out of Memory] base::`anonymous namespace'::oom_killer_malloc)

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

- Go/Fracas
Cc: ychiu@google.com
 Issue 673982  has been merged into this issue.
Looks like a Mac AppKit crash, removing RBS, feel free to add if needed.
Labels: -ReleaseBlock-Stable
Project Member

Comment 64 by sheriffbot@chromium.org, Dec 16 2016

Labels: FoundIn-M-56
Users experienced this crash on the following builds:

Mac Beta 56.0.2924.28 -  0.79 CPM, 4 reports, 2 clients (signature [Out of Memory] base::`anonymous namespace'::oom_killer_malloc)

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

- Go/Fracas
Labels: Fracas-Wrong
Issue 675638 has been merged into this issue.
Issue 675952 has been merged into this issue.
Still experiencing crash on 57.0.2970.0 (Official Build) dev (64-bit)
hi,  @ranjitkan hope to fix it soon. we have more and more user meet this crash. Thanks so much.
 Issue 692389  has been merged into this issue.
Issue 693976 has been merged into this issue.
hi rsesek, as more mac sierra users. Hope fix this issue soon.
Thanks so much.

Cc: hdodda@chromium.org
 Issue 693829  has been merged into this issue.
Labels: Restrict-AddIssueComment-EditIssue
This is an Apple bug. See https://bugs.chromium.org/p/chromium/issues/detail?id=654695#c45. Feel free to notify Apple about your continued interest in seeing a fix to this issue.

Comment 75 by kochi@chromium.org, Feb 24 2017

Cc: krajshree@chromium.org
 Issue 689854  has been merged into this issue.

Comment 76 by kochi@chromium.org, Feb 24 2017

Cc: ctengc...@gmail.com
Issue 694481 has been merged into this issue.
Issue 696923 has been merged into this issue.
Issue 702453 has been merged into this issue.
Issue 703913 has been merged into this issue.
Issue 703956 has been merged into this issue.
Issue still seen on Mac stable 57.0.2987.110 as top # 1 browser crash having 710 crashes from 310 unique client ids.

57.0.2987.110	2.75%	710	--M57(1% Stable)
57.0.2987.98	9.16%	2365	
56.0.2924.87	85.99%	22212	--M56(100% Stable)
56.0.2924.76	0.16%	41	
55.0.2883.95	1.39%	359	
55.0.2883.87	0.19%	48	
55.0.2883.75	0.02%	6	
54.0.2840.98	0.26%	67	
54.0.2840.87	0.06%	16	
54.0.2840.71	0.03%	8	
Issue 706403 has been merged into this issue.
Issue 706330 has been merged into this issue.
Issue 711853 has been merged into this issue.
Issue 711875 has been merged into this issue.
Cc: -karandeepb@chromium.org

Comment 88 by ajha@chromium.org, Apr 21 2017

Issue 713015 has been merged into this issue.
Issue 716386 has been merged into this issue.

Comment 90 by kbr@chromium.org, May 2 2017

Cc: kbr@chromium.org
Still happening in Chrome 58. A family member ran into this while composing an article online. More Crash IDs:

c7466ac730000000
8fcdfc1390000000
3731821390000000
6515fc1390000000
3932821390000000
51e6021390000000
78172fbb30000000

I'll push Apple on the Radar.

 Issue 717861  has been merged into this issue.
Issue 717980 has been merged into this issue.
Issue 721143 has been merged into this issue.
Issue 671595 has been merged into this issue.
Issue 728020 has been merged into this issue.
Issue 728768 has been merged into this issue.
Issue 730302 has been merged into this issue.
The word from Apple is that this bug is fixed in 10.13 - is there a way for us to confirm?
Owner: manoranj...@chromium.org
I can verify this once OS X 10.13 set-up is ready.
 Issue 734614  has been merged into this issue.
 Issue 734644  has been merged into this issue.
Issue 734646 has been merged into this issue.
Issue 733454 has been merged into this issue.

Comment 104 by kbr@chromium.org, Jun 20 2017

Is there any possible workaround on the Chrome side until 10.13 is released and widespread? If it's possible to narrowly identify the region where Apple's code is performing the illegal memory allocation, could we perhaps disable the OOM killer just for that brief period of time? This seems to be affecting a lot of Chrome users who use Chinese keyboards on a regular basis.

Instructions:
"""
Steps to Reproduce:
1) Open the attached xcode-project.
2) Make sure that ASAN [address sanitizer] is enabled.
3) Compile and run the program.
4) Left click the red box.
5) Make sure that [Pinyin - simplified] IME is enabled.
6) Type "q"
"""
Labels: TE-Verified-OSX-10.13
erikchen@, thank you for providing the repro steps.

I've verified the above "IME_test" (c#45) on OS X 10.13 Beta and it's working fine without any issues.

PS: I'm able to reproduce the crash from same 'IME_Test' consistently on OS X 10.12.x.

Thank you!
Issue 725378 has been merged into this issue.
Issue 737823 has been merged into this issue.
Issue 736636 has been merged into this issue.
Issue 740368 has been merged into this issue.
 Issue 741247  has been merged into this issue.
Just wanted to point out that  issue 741247  introduces a reliable repro of the crash which I verified on content shell (as well as chrome).

Following comment #27, the issues in RWHVCocoa's implementatio along with AppKit's bug leads to the crash. I did try the same steps in some other apps in Mac as well as chrome's omnibox (which I believe is based on StyledTextField: NSTextField) but none of the other cases crashed...so ,in theory, there might be a fix for the crash on our end.
> Just wanted to point out that   issue 741247   introduces a reliable repro of the crash which I verified on content shell (as well as chrome).

Have you tried it on 10.13?

Hmm...no my macOS is 10.12.5. I have not tested it on 10.13 but based on comment #106 I believe it should not crash.
Issue 742428 has been merged into this issue.
Issue 748756 has been merged into this issue.
 Issue 757864  has been merged into this issue.
Issue 757981 has been merged into this issue.
Project Member

Comment 119 by bugdroid1@chromium.org, Aug 24 2017

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

commit 9984518a9c261026b5d2c4b82c7fd7372a06f1c2
Author: Robert Sesek <rsesek@chromium.org>
Date: Thu Aug 24 20:53:33 2017

[Mac] Swizzle IMKInputSession to temporarily disable fatal OOM.

On macOS 10.12, an IMKInputSession will attempt to allocate a 2^64
buffer, which will always crash because OOM is made fatal by the
allocator shims. Temporarily disable this behavior by swizzling the
faulty method, setting a flag to disable fatal OOM, and then calling
the original.

Bug:  654695 
Change-Id: I3213301cab9b997c8c1b31cd058c4c6baefce684
Reviewed-on: https://chromium-review.googlesource.com/631577
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497184}
[modify] https://crrev.com/9984518a9c261026b5d2c4b82c7fd7372a06f1c2/chrome/browser/app_controller_mac.mm

Issue 748880 has been merged into this issue.
Project Member

Comment 121 by bugdroid1@chromium.org, Aug 24 2017

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

commit 1eb85221f8f0805a5172ea603dddc706fd4d90a4
Author: Robert Sesek <rsesek@chromium.org>
Date: Thu Aug 24 23:17:09 2017

Fix Mac ASan build after 9984518a9c261026b5d2c4b82c7fd7372a06f1c2.

No-Try: True
Tbr: erikchen@chromium.org
Bug:  654695 
Change-Id: I6d03a4e44edc590cc42f8c7c60fd2fd9eaec8cc3
Reviewed-on: https://chromium-review.googlesource.com/633835
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497238}
[modify] https://crrev.com/1eb85221f8f0805a5172ea603dddc706fd4d90a4/chrome/browser/app_controller_mac.mm

Issue 754262 has been merged into this issue.
Cc: manoranj...@chromium.org
Labels: -M-56 M-62
Owner: rsesek@chromium.org
Status: Fixed (was: ExternalDependency)
Workaround is in M62, and Apple have a fix in 10.13. Going to call this fixed.
Issue 734451 has been merged into this issue.
Issue 763218 has been merged into this issue.
Issue 773531 has been merged into this issue.
Issue 773660 has been merged into this issue.
Chrome 62 is starting to roll out to stable channel, so this workaround fix will be generally available soon.

https://chromereleases.googleblog.com/2017/10/stable-channel-update-for-desktop.html
Issue 669524 has been merged into this issue.
 Issue 780486  has been merged into this issue.
Issue 777202 has been merged into this issue.
Showing comments 32 - 131 of 131 Older

Sign in to add a comment