1512165 - Crash in sogoutsf.ime@0x474a0

Status: RESOLVED WORKSFORME

(Core :: Widget: Win32, defect)

Description

[[:philipp]]

This bug was filed from the Socorro interface and is
report bp-0a35b037-9c93-4a3b-b632-179ec0181124.
=============================================================

Top 10 frames of crashing thread:

0 sogoutsf.ime sogoutsf.ime@0x474a0 
1 sogoutsf.ime sogoutsf.ime@0x47959 
2 sogoutsf.ime sogoutsf.ime@0x47346 
3 msctf.dll CInputContext::_DoEditSession 
4 msctf.dll CInputContext::_EditSessionQiCallback 
5 msctf.dll CInputContext::_EmptyLockQueue 
6 msctf.dll CACPWrap::OnLockGranted 
7 xul.dll mozilla::widget::TSFTextStore::RequestLock widget/windows/TSFTextStore.cpp:2305
8 msctf.dll CACPWrap::RequestLock 
9 msctf.dll CInputContext::_QueueItem 

=============================================================

crash reports like this one with the Sogou Pinyin input method have become more common during the 65 nightly cycle. they are all from installations on windows 10 with a11y active.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Looks like that at least the stack in comment 0 is caused by internal bug of Sogou Pinyin because we grabs necessary objects with local variables correctly when calling ITfKeystrokeMgr::KeyDown() and ITextStoreACPSink::OnLockGranted() but Sogou Pinyin references nullptr.

I guess that Sogou reads web content or something with a11y API. A Japanese 3rd party IME, ATOK, also does use a11y API to read content in focused editor to improve suggestion from IME. But perhaps, a11y is not related to this crash.

Last update of Sogou Pinyin is 2018-12-5. So, it must be new regression of them. (Or, we are just victim of new race condition between treads. We experienced such issue of Japanese Microsoft IME in this Q1 and Q2.)

Comment 2

[Julien Cristau [:jcristau]]

Do we have contacts with Sogou?

Comment 3

[[:philipp]]

looking at the crashing graph for beta it's only correlating with the rollout of 65 to the beta population, so i think it's likely something in our codebase that's triggering the crashes more often since 65...

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

We didn't touch TSFTextStore in 65 cycle...

Comment 5

[[:philipp]]

are these crashes reproducible while using Sogou Pinyin and if so, would it be possible to come up with a regression range for the issue?

thank you

Comment 6

[yxu]

sorry for the late reply. I am trying to reproduce this question, but I don't see it clearly related to 65beta? There is currently no user feedback on this issue. I have previously contacted the Sogou input method because of other problems. They used a11y's screen reading function to analyze the content of the web page and input environment to complete. I'm not sure if this is relevant, but this feature should be used very early.

Comment 7

[[:philipp]]

I don't see it clearly related to 65beta

during the 64.0b cycle there were a total of 44 crash reports with involvement of sogou (0.02% of all browser process crashes), while in 65.0b there are already over 1k reports at this point (1.25% of all browser crashes).

the first appearance in the 65 nightly cycle was from 65.0a1 build 20181028102553 and reports are coming in pretty regularly thereafter. in this potential regression range, these would be the changes on nightly around that timeframe (october 25 till build 20181028102553): https://mzl.la/2GYxD3R

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

Jeff, could this be related to bug 1502286?

Comment 9

[Kelsey Gilbert [:jgilbert]]

Seems unlikely, since that bug resulted in a backout, reverting to the previous state.

Comment 10

[[:philipp]]

there are also 3 a11y related changes in the potential regression range - perhaps they are contributing to the problem?

Comment 11

[Andrew Overholt [:overholt]]

(In reply to [:philipp] from comment #10)

there are also 3 a11y related changes in the potential regression range - perhaps they are contributing to the problem?

Maybe Jamie can take a quick look.

Comment 12

[Andrew Overholt [:overholt]]

Given we have so little to go on, it seems a fix on Firefox's side is unlikely in 64 or 65.

Comment 13

[James Teh [:Jamie]]

I don't see how this could be related to any of those a11y changes (unless there is some very obscure condition in Sogou, in which case there's nothing we can do):

• Bug 1501496 is Android only.
• Bug 1052866 exposes an additional COM interface on tables and rows. I doubt Sogou is querying for it, and even if it is, it can't get a nullptr. Also, we already exposed that interface on other accessibles.
• Bug 1501595 changes a role we expose. For IAccessible::accRole (which Sogou is most likely using), there is no change. For IAccessible2::role, we just return a different int.

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

At least, sogoutsf.ime@0x537f, sogoutsf.ime@0x6a5d5, sogoutsf.ime@0x52cf and sogoutsf.ime@0x52ff are obviously their bugs because we're not in the stack. And all of them refers nullptr. So, they just forget nullptr-check before referring smart or raw pointer.

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

And all of them refers nullptr.

Ah, all crashes managed by this bug are caused by referring nullptr.

Comment 16

[[:philipp]]

the volume of this crash on the beta channel has reduced remarkably after jan 23, so i assume that the third-party vendor might have released some sort of fix. anyway, we don't need to monitor this any longer...