595097 - crash opening/saving attachment [@ nsCounterManager::AddCounterResetsAndIncrements(nsIFrame*)] MODULE: psicon.dll [adobe photoshop?]

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Ludovic Hirlimann [:Usul]]

+++ This bug was initially created as a clone of Bug #522575 +++

crash [@ nsCounterManager::AddCounterResetsAndIncrements(nsIFrame*)]


two crashes mention opening an attachment
http://crash-stats.mozilla.com/report/index/93cffb1c-fa4d-4409-817f-54f672090729
0	thunderbird.exe	nsCounterManager::AddCounterResetsAndIncrements	 layout/base/nsCounterManager.cpp:220
1	thunderbird.exe	nsCSSFrameConstructor::InitAndRestoreFrame	layout/base/nsCSSFrameConstructor.cpp:6807
2	thunderbird.exe	nsCSSFrameConstructor::ConstructXULFrame	layout/base/nsCSSFrameConstructor.cpp:6175
3	thunderbird.exe	nsCSSFrameConstructor::ConstructFrameInternal	layout/base/nsCSSFrameConstructor.cpp:7574
4	thunderbird.exe	nsCSSFrameConstructor::ConstructFrame	layout/base/nsCSSFrameConstructor.cpp:7444
5	thunderbird.exe	nsCSSFrameConstructor::ProcessChildren	layout/base/nsCSSFrameConstructor.cpp:11448
6	thunderbird.exe	nsCSSFrameConstructor::ConstructXULFrame	layout/base/nsCSSFrameConstructor.cpp:6240
7	thunderbird.exe	nsCSSFrameConstructor::ConstructFrameInternal	layout/base/nsCSSFrameConstructor.cpp:7574
8	thunderbird.exe	nsCSSFrameConstructor::ConstructFrame	layout/base/nsCSSFrameConstructor.cpp:7444
9	thunderbird.exe	nsCSSFrameConstructor::ProcessChildren	layout/base/nsCSSFrameConstructor.cpp:11448
10	thunderbird.exe	nsCSSFrameConstructor::ConstructXULFrame	layout/base/nsCSSFrameConstructor.cpp:6240
11	thunderbird.exe	nsCSSFrameConstructor::ConstructFrameInternal	layout/base/nsCSSFrameConstructor.cpp:7574
12	thunderbird.exe	nsCSSFrameConstructor::ConstructFrame	layout/base/nsCSSFrameConstructor.cpp:7444
13	thunderbird.exe	nsCSSFrameConstructor::ProcessChildren	layout/base/nsCSSFrameConstructor.cpp:11448
14	thunderbird.exe	nsCSSFrameConstructor::ConstructXULFrame	layout/base/nsCSSFrameConstructor.cpp:6240
15	thunderbird.exe	nsCSSFrameConstructor::ConstructFrameInternal	layout/base/nsCSSFrameConstructor.cpp:7574
16	thunderbird.exe	nsCSSFrameConstructor::ConstructFrame	layout/base/nsCSSFrameConstructor.cpp:7444
17	thunderbird.exe	nsCSSFrameConstructor::ContentInserted	layout/base/nsCSSFrameConstructor.cpp:9056
18	thunderbird.exe	nsCSSFrameConstructor::ContentAppended	layout/base/nsCSSFrameConstructor.cpp:8556
19	thunderbird.exe	PresShell::ContentAppended	layout/base/nsPresShell.cpp:4993
20	thunderbird.exe	nsNodeUtils::ContentAppended	content/base/src/nsNodeUtils.cpp:120
21	thunderbird.exe	nsGenericElement::doInsertChildAt	content/base/src/nsGenericElement.cpp:3277 

http://crash-stats.mozilla.com/report/list?range_value=2&range_unit=weeks&signature=nsCounterManager%3A%3AAddCounterResetsAndIncrements%28nsIFrame*%29&version=Thunderbird%3A3.1.3

Opening because the dll with thought was causing this is now blocked. Few comments are talking about opening attachments.

Comment 1

[Wayne Mery (:wsmwk)]

#2 crash for TB 3.1.4, so nsCounterManager::AddCounterResetsAndIncrements really needs attention/analysis.


ludo (or someone), please compare this bug's stacks to see if the following is same crasher ...

nsRuleNode::GetStyleContent(nsStyleContext*, int)] 
MODULE: psicon.dll is loaded
all crashes in last 4 months are v3.1.x except 1, so, unlike nsCounterManager::AddCounterResetsAndIncrements, this stack signature is new for 1.9.2. 

bp-06d285a8-a9c3-4668-a120-76db92100830 (sitedesign)
bp-8d24ffb1-8679-4154-8305-3ba632100827 (rcgarner)
bp-a190b3cb-8148-4001-a32e-ba2a42100910 (cowchip)
Last Crash	38 seconds before submission
Install Age	220811 seconds (2.6 days) since version was first installed.
Version	3.1.3
User Comments	happends whenever I open a jpg attached picture. only with the latest ver of thunderbird. never happend before

1       thunderbird.exe	nsStyleContext::GetStyleContent	layout/style/nsStyleStructList.h:105
2	thunderbird.exe	nsCounterManager::AddCounterResetsAndIncrements	layout/base/nsCounterManager.cpp:225
3	thunderbird.exe	nsCSSFrameConstructor::InitAndRestoreFrame	layout/base/nsCSSFrameConstructor.cpp:4770
4	thunderbird.exe	nsCSSFrameConstructor::ConstructFrameFromItemInternal	layout/base/nsCSSFrameConstructor.cpp:3972
5	thunderbird.exe	nsCSSFrameConstructor::ConstructFramesFromItem	layout/base/nsCSSFrameConstructor.cpp:5647
6	thunderbird.exe	nsCSSFrameConstructor::ConstructFramesFromItemList	layout/base/nsCSSFrameConstructor.cpp:9636
7	thunderbird.exe	nsCSSFrameConstructor::ProcessChildren	layout/base/nsCSSFrameConstructor.cpp:9744
8	thunderbird.exe	nsCSSFrameConstructor::ConstructFrameFromItemInternal	layout/base/nsCSSFrameConstructor.cpp:4018
9	thunderbird.exe	nsCSSFrameConstructor::ConstructFramesFromItem	layout/base/nsCSSFrameConstructor.cpp:5647
10	thunderbird.exe	nsCSSFrameConstructor::ConstructFramesFromItemList	layout/base/nsCSSFrameConstructor.cpp:9636
11	thunderbird.exe	nsCSSFrameConstructor::ProcessChildren	layout/base/nsCSSFrameConstructor.cpp:9744
12	thunderbird.exe	nsCSSFrameConstructor::ConstructFrameFromItemInternal	layout/base/nsCSSFrameConstructor.cpp:4018
13	thunderbird.exe	nsCSSFrameConstructor::ConstructFramesFromItem	layout/base/nsCSSFrameConstructor.cpp:5647
14	thunderbird.exe	nsCSSFrameConstructor::ConstructFramesFromItemList	layout/base/nsCSSFrameConstructor.cpp:9636
15	thunderbird.exe	nsCSSFrameConstructor::ProcessChildren	layout/base/nsCSSFrameConstructor.cpp:9744
16	thunderbird.exe	nsCSSFrameConstructor::ConstructFrameFromItemInternal	layout/base/nsCSSFrameConstructor.cpp:4018
17	thunderbird.exe	nsCSSFrameConstructor::ConstructFramesFromItem	layout/base/nsCSSFrameConstructor.cpp:5647
18	thunderbird.exe	nsCSSFrameConstructor::ConstructFramesFromItemList	layout/base/nsCSSFrameConstructor.cpp:9636
19	thunderbird.exe	nsCSSFrameConstructor::ContentInserted	layout/base/nsCSSFrameConstructor.cpp:6876
20	thunderbird.exe	nsCSSFrameConstructor::ContentAppended	layout/base/nsCSSFrameConstructor.cpp:6351
21	thunderbird.exe	PresShell::ContentAppended	layout/base/nsPresShell.cpp:5044
22	thunderbird.exe	nsNodeUtils::ContentAppended	content/base/src/nsNodeUtils.cpp:136
23	thunderbird.exe	nsGenericElement::doInsertChildAt	content/base/src/nsGenericElement.cpp:3238

Comment 2

[Ludovic Hirlimann [:Usul]]

(In reply to comment #1)
> #2 crash for TB 3.1.4, so nsCounterManager::AddCounterResetsAndIncrements
> really needs attention/analysis.
> 
> 
> ludo (or someone), please compare this bug's stacks to see if the following is
> same crasher ...
> 
> nsRuleNode::GetStyleContent(nsStyleContext*, int)] 
> MODULE: psicon.dll is loaded
It can't be or something is badly broken. psicon.dll is now blocked from being loaded in TB 3.1.4 - so this should have been fixed. It's not. Wayne could you contact only the people crashing on 3.1.4, and try to gather more system information - do they have a common piece of software that is installed on their machine etc ...

Standard8 are we sure that the dll blocking mechanism works ?

Comment 3

[LegNeato]

I'm going to not block a FF release waiting for this, though we certainly would take a fix. Marking as wanted.

Comment 4

[Wayne Mery (:wsmwk)]

(In reply to comment #2)
> It can't be or something is badly broken. psicon.dll is now blocked from being
> loaded in TB 3.1.4 - so this should have been fixed. It's not. Wayne could you
> contact only the people crashing on 3.1.4, and try to gather more system
> information - do they have a common piece of software that is installed on
> their machine etc ...

yeah. they all have psicon.dll      6.6.64.53

> Standard8 are we sure that the dll blocking mechanism works ?

afaict, bug 522575 was not verified via anything other than code inspection.
clearly, these crashes are still going strong
v3.1.4 https://crash-stats.mozilla.com/report/list?product=Thunderbird&version=Thunderbird%3A3.1.4&build_id=&query_search=signature&query_type=exact&query=nsCounterManager%3A%3AAddCounterResetsAndIncrements%28nsIFrame*%29&date=09%2F22%2F2010%2010%3A43%3A57&range_value=4&range_unit=weeks&hang_type=any&process_type=any&plugin_field=&plugin_query_type=&plugin_query=&do_query=1&signature=nsCounterManager%3A%3AAddCounterResetsAndIncrements%28nsIFrame*%29&missing_sig=&page=1
v3.1.3 http://crash-stats.mozilla.com/query/query?product=Thunderbird&version=Thunderbird%3A3.1.3&range_value=4&range_unit=weeks&date=09%2F22%2F2010+10%3A43%3A57&query_search=signature&query_type=exact&query=nsCounterManager%3A%3AAddCounterResetsAndIncrements%28nsIFrame*%29&build_id=&process_type=any&hang_type=any&do_query=1

Comment 5

[Wayne Mery (:wsmwk)]

is this a layout/css issue?

(In reply to comment #4)
> yeah. they all have psicon.dll      6.6.64.53

however, there are a few (near zero) Mac firefox crashes, that have no psicon
bp-ad08f47c-7e6c-443c-87ef-6fd032100906 10.5.8 9L31a
bp-9311bf03-39d2-47db-b3f7-e566f2100907 10.6.4 10F569
bp-8cc5ce2c-2801-4d29-98ff-b22f32100715 10.6.4 10F569 (ivie-) has an email address

bp-947d6239-81bf-4f3e-a227-f85e42100725 windows 3.6.8, reporter claims 3.6.7 did not crash - interesting, but all firefox versions do crash
0	xul.dll	nsCounterManager::AddCounterResetsAndIncrements	 layout/base/nsCounterManager.cpp:227
1	xul.dll	nsCSSFrameConstructor::ConstructFrameFromItemInternal	layout/base/nsCSSFrameConstructor.cpp:3972
2	xul.dll	nsCSSFrameConstructor::ConstructFramesFromItem	layout/base/nsCSSFrameConstructor.cpp:5647
3	xul.dll	nsCSSFrameConstructor::ProcessChildren	layout/base/nsCSSFrameConstructor.cpp:9744
4	xul.dll	nsCSSFrameConstructor::ConstructFrameFromItemInternal	layout/base/nsCSSFrameConstructor.cpp:4018 


(rare) variation...
nsCounterManager::AddResetOrIncrement(nsIFrame*, int, nsStyleCounterData const*, nsCounterNode::Type)
bp-e66ba781-0f05-43a5-b31e-211dd2100926
bp-0579bb68-c68d-4b9c-b3bf-d1dc62100927
0    xul.dll    nsCounterManager::AddResetOrIncrement     layout/base/nsCounterManager.cpp:251
1    xul.dll    nsCounterManager::AddCounterResetsAndIncrements    
2    xul.dll    nsCSSFrameConstructor::ConstructFrameFromItemInternal    layout/base/nsCSSFrameConstructor.cpp:3972
3    xul.dll    nsCSSFrameConstructor::ConstructFramesFromItem    layout/base/nsCSSFrameConstructor.cpp:5647
4    xul.dll    nsCSSFrameConstructor::ConstructFramesFromItemList    layout/base/nsCSSFrameConstructor.cpp:9636
5    xul.dll    nsCSSFrameConstructor::ContentInserted    layout/base/nsCSSFrameConstructor.cpp:6876
6    xul.dll    nsCSSFrameConstructor::RecreateFramesForContent    layout/base/nsCSSFrameConstructor.cpp:9255

Comment 6

[timeless]

ok, that first mac one is clearly a null pointer, but someone said that the getter shouldn't be giving those out.

psicon is of course w32 only (it's an icon handler which loads if you use a common dialog [open/save])

Comment 7

[timeless]

FF bug 522575 fixed - 8/31 (trunk), 9/20 (1.9.2)
TB bug 596196 fixed ~ 9/15 (trunk), 9/20 (1.9.2)

the following are all crashes from before the fixed dates above:
bp-947d6239-81bf-4f3e-a227-f85e42100725 FF 1.9.2(7/22) crashed 7/25
bp-06d285a8-a9c3-4668-a120-76db92100830 TB 1.9.2(8/2) crashed 8/30
bp-8d24ffb1-8679-4154-8305-3ba632100827 TB 1.9.2(8/2) crashed 8/27
bp-a190b3cb-8148-4001-a32e-ba2a42100910 TB 1.9.2(8/25) crashed 9/10

the following are 1.9.1 branch, afaict no one has adjusted the blocklist for that branch:
bp-e66ba781-0f05-43a5-b31e-211dd2100926
bp-93cffb1c-fa4d-4409-817f-54f672090729

bp-0579bb68-c68d-4b9c-b3bf-d1dc62100927 doesn't have psicon.

So, we have 3 major buckets "crashed before fixed", "crashed in unfixed branch [fixing should be easy?]", "rare, unexplained".

I vote for fixing 1.9.1 and trying to monitor for cases which are crashes *after* fix :)

Comment 8

[Mark Banner (:standard8)]

(In reply to comment #7)
> I vote for fixing 1.9.1 and trying to monitor for cases which are crashes
> *after* fix :)

1.9.1 doesn't have the binary blocklist capability.

Comment 9

[timeless]

Well that's rather unfortunate. Can you try not to complain about them then? :)

Comment 10

[Mark Banner (:standard8)]

(In reply to comment #6)
> ok, that first mac one is clearly a null pointer, but someone said that the
> getter shouldn't be giving those out.

xref bug 522575 comment 21.

Whilst the statement is that the getter shouldn't be giving these out, there's actually nothing to stop it. Nor does the object create the item itself.

I don't know the code well enough, but just reading around these frames:

nsCounterManager::AddCounterResetsAndIncrements    
layout/base/nsCounterManager.cpp:227
nsCSSFrameConstructor::ConstructFrameFromItemInternal   
layout/base/nsCSSFrameConstructor.cpp:3972
nsCSSFrameConstructor::ConstructFramesFromItem   
layout/base/nsCSSFrameConstructor.cpp:5647
nsCSSFrameConstructor::ProcessChildren   
layout/base/nsCSSFrameConstructor.cpp:9744

I can see that GetStyleContent() will return whatever is set in the Item being constructed from, even if that is null.

Unfortunately I don't know where to look next but I would say there's a definite possibility here depending on how those Items are being put together.

Comment 11

[Wayne Mery (:wsmwk)]

#1 crash for Tbird 3.1.9  (excluding the new installer crash)

Comment 12

[Wayne Mery (:wsmwk)]

workaround from bug 522575 comment 31

feedback I received from the Photoshop team:
Yes, psicon.dll was a shell extension to provide explorer thumbnails on
Windows. We got rid of psicon.dll several years ago  (CS3 I think) — it relied
on Windows APIs that caused files to remain open for reading (and prevent apps
from saving over them).  In other cases, the OS APIs would cause crashes while
rendering the thumbnails. As far as we know, Microsoft never fixed those APIs.
(the bug is still open, still marked as pending fix)

To resolve this, you should be able to delete or rename the file.

Comment 13

[Wayne Mery (:wsmwk)]

ludo, standard8,

further action is needed. can someone get to the bottom please?

IMO the blocklist in bug 522575 is ineffective for thunderbird. indeed it is now #1 crash for Thunderbird v3.1.9. 

I sampled about a dozen 3.1.9 crashes and all have psicon.dll in the module list. Also, note that some PC can have multiple psicon.dll files as experienced in http://getsatisfaction.com/mozilla_messaging/topics/crashes_induced_by_saving_opening_attachment_getting_old#reply_5205255

Comment 14

[timeless]

wayne: per comment 8 it's ineffective because the feature (binary blocklist) doesn't exist in 1.9.1. that isn't a bug in the blocklist, it's a lack of a backported feature.

Comment 15

[Wayne Mery (:wsmwk)]

(In reply to comment #14)
> wayne: per comment 8 it's ineffective because the feature (binary blocklist)
> doesn't exist in 1.9.1. that isn't a bug in the blocklist, it's a lack of a
> backported feature.

Yup. Except this bug is, I think, not (only) about gecko 1.9.1, and my writings (and comment 0) have been about version 3.1/ 1.9.2. Perhaps there is confusion because comment 0 doesn't say in words whether this bug is about trunk or version 3.1/gecko 1.9.2. (and the version field is unspecified)


So, what is troublesome is bug 522575 by all accounts fixed the issue for FF 3.6, which is also 1.9.2-based, but TB 3.1 has not been helped. 

ludo, standard8, do you recall whether 3.1.3 was helped in any way?  We have no bug comments in bug 522575 which indicate there was or was not a drop off in 3.1.3 crashes after bug 522575 comment 39 checkin.  

ludo, do you recall was found in your 3.1.3 QA at the time?

Comment 16

[timeless]

yeah, sorry, i had assumed that 3.1.x was 1.9.1

Comment 17

[Daniel Veditz [:dveditz]]

Ehsan: any idea why the dll blocklisting in bug 522575 fixed Firefox 3.6 but didn't help Thunderbird?

Comment 18

[(no longer active)]

(In reply to comment #17)
> Ehsan: any idea why the dll blocklisting in bug 522575 fixed Firefox 3.6 but
> didn't help Thunderbird?

Yes.  The blocklist added in bug 522575 landed for 1.9.2.11.  The crashes for Thunderbird happen until 1.9.2.9, so in fact that blocklist has helped Thunderbird as well.

<https://crash-stats.mozilla.com/report/list?product=Thunderbird&query_search=signature&query_type=contains&query=AddCounterResetsAndIncrements&date=04%2F13%2F2011%2013%3A29%3A45&range_value=1&range_unit=weeks&hang_type=any&process_type=any&plugin_field=&plugin_query_type=&plugin_query=&do_query=1&admin=&signature=nsCounterManager%3A%3AAddCounterResetsAndIncrements%28nsIFrame*%29>

Comment 19

[Mark Banner (:standard8)]

I've just taken a look at this and surprisingly we're missing a define which ends up meaning the dll blocklist isn't working for Thunderbird. I'm working on testing a patch to fix this.

Comment 20

[Mark Banner (:standard8)]

Attachment: Make the DLL blocklist work

This should make the blocklist work as it copies the same set-up as Firefox & SeaMonkey. My build is still running but I'll do a manual test once it finishes and before I land the patch.

Comment 21

[Mark Banner (:standard8)]

Ok, I manually verified this by adding some extra entries to the blocklist in my debug build.

Checked in:

http://hg.mozilla.org/comm-central/rev/39980b40c407

Comment 22

[Mark Banner (:standard8)]

Comment on attachment 525824 [details] [diff] [review]
Make the DLL blocklist work

We're going to take this for 3.1.10 as it should be low risk and it is our top crasher.

Comment 23

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/releases/comm-1.9.2/rev/8f80fb4514a0

Comment 24

[Steve Snyder]

(In reply to comment #22)
> Comment on attachment 525824 [details] [diff] [review]
> Make the DLL blocklist work
> 
> We're going to take this for 3.1.10 as it should be low risk and it is our top
> crasher.

The comments above suggest that the fix should be in the v3.1.10 candidate builds.  Actually, the patch has not been applied, at least not to the Build #1 source tarball.

ftp://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/3.1.10-candidates/build1/source/thunderbird-3.1.10.source.tar.bz2

Comment 25

[Mark Banner (:standard8)]

(In reply to comment #24)
> The comments above suggest that the fix should be in the v3.1.10 candidate
> builds.  Actually, the patch has not been applied, at least not to the Build #1
> source tarball.

We messed up on the build config and we're working on fixing it. Please don't assume a build is valid to any extent until we announce it.

Comment 26

[Rob]

Can we change the "Status:" to REOPENED please, or would you prefer a separate Report for FF (vs. Thunderbird).


Here is a recent crash on a "Release" version of FF:
...
Install Time	2011-10-01 01:34:01
Product		Firefox
Version		7.0.1
Build ID	20110928134238
Release Channel	release
OS		Windows NT
...
bp-936beb2e-e719-4894-9d86-2cf482111108.

Comment 27

[Mark Banner (:standard8)]

(In reply to Rob from comment #26)
> Can we change the "Status:" to REOPENED please, or would you prefer a
> separate Report for FF (vs. Thunderbird).

Please file a separate report.

> Here is a recent crash on a "Release" version of FF:
> ...
> Install Time	2011-10-01 01:34:01
> Product		Firefox
> Version		7.0.1
> Build ID	20110928134238
> Release Channel	release
> OS		Windows NT
> ...
> bp-936beb2e-e719-4894-9d86-2cf482111108.

Of note, I don't see psicon.dll in the module list for that crash, so there is probably a different crash source.