84767 - Crash in ConvertMoveToFolderValue logging into mail server

Status: VERIFIED FIXED

(MailNews Core :: Filters, defect, P2)

Description

[Brian Nesse (gone)]

Fairly regularly I'm getting crashes while logging into mail (I've filed numerous 
talkbacks on it). The crashes are always in the filter code, which is odd because 
I have never used mail filters, and generally the only way I can fix it is by 
creating a new profile.

Here is the stack crawl from my latest crash:

 Calling chain using A6/R1 links
  Back chain  ISA  Caller
  00000000    PPC  0EA7F910  
  0AD9E890    PPC  0EA61DF0  main+001AC
  0AD9E830    PPC  0EA5EFF0  main1(int, char**, nsISupports*)+009B4
  0AD9E5C0    PPC  0E431BBC  nsAppShellService::Run()+00054
  0AD9E570    PPC  0E3D6178  nsAppShell::Run()+0004C
  0AD9E530    PPC  0E3D6D80  nsMacMessagePump::DoMessagePump()+00044
  0AD9E4E0    PPC  0E3D76F8  nsMacMessagePump::DispatchEvent(int, EventRecord*)+
001B0
  0AD9E490    PPC  0E3F6D04  Repeater::DoRepeaters(const EventRecord&)+0003C
  0AD9E440    PPC  0E3BD990  nsMacNSPREventQueueHandler::RepeatAction(const 
EventRecord&)+00014
  0AD9E400    PPC  0E3BDFA0  nsMacNSPREventQueueHandler::ProcessPLEventQueue()+
00180
  0AD9E380    PPC  0E8CD7B0  nsEventQueueImpl::ProcessPendingEvents()+00068
  0AD9E310    PPC  0E9409FC  PL_ProcessPendingEvents+000BC
  0AD9E2C0    PPC  0E940C50  PL_HandleEvent+00054
  0AD9E280    PPC  0E905FB8  EventHandler(PLEvent*)+00074
  0AD9E230    PPC  0E8FBE70  XPTC_InvokeByIndex+0002C
  0AD9E1F0    PPC  0E8FBF78  _XPTC_InvokeByIndex+000C8
  0AD9E13C    PPC  0DCDBBF4  
nsImapMailFolder::SetupHeaderParseStream(nsIImapProtocol*, unsigned int, const 
char*, nsIMailboxSpec*)+001B0
  0AD9E07C    PPC  0DD6F7DC  nsMsgIncomingServer::GetFilterList(nsIMsgFilterList*
*)+007FC
  0AD9DF4C    PPC  0AE3B7A8  nsMsgFilterService::OpenFilterList(nsIFileSpec*, 
nsIMsgFolder*, nsIMsgFilterList**)+002A4
  0AD9DE3C    PPC  0AE27610  nsMsgFilterList::LoadTextFilters(nsIOFileStream*)+
0032C
 Closing log

It is dying in "case nsIMsgFilterList::attribActionValue:" in the code:
  if (m_curFilter->m_action.m_type == nsMsgFilterAction::MoveToFolder)
    err = m_curFilter->ConvertMoveToFolderValue(value);

because m_curFilter is NULL. Most of the other cases in this switch statement 
check if (m_curFilter) before accessing it... the attribAction and 
attribActionValue cases don't.

Comment 1

[Brian Nesse (gone)]

walking through the process I get, in sequence:
attribVersion
attribLogging
attribEnabled
attribType
AttribAction      <- writes to null object, stomps on memory
AttribActionValue <- jumps  to null object, crashes

Is there always supposed to be a valid m_curFilter, or do these last two cases 
need to be protecting themselves?

Comment 2

[Håkan Waara]

Brian, do you have any testcase to reproduce this?

I've never seen this crash before.

Comment 3

[Brian Nesse (gone)]

I can reproduce it on my development machine 100% of the time. I assume that
it's related to my profile contents... more specifically probably my prefs.js
file contents. But that's only a guess and I haven't attempted to narrow it down.

Comment 4

[Håkan Waara]

Can you post your rules.dat file please?

Comment 5

[Brian Nesse (gone)]

Attachment: Brian's rules.dat file

Comment 6

[Navin Gupta]

The last two cases need to be protected by checking for null. nominating

Comment 7

[Navin Gupta]

Attachment: bullet proofing for the last two cases

Comment 8

[Navin Gupta]

Looking at your rules.dat file there is no name so m_curFilter is never
allocated any memory. Therefore we need to protect for null for these last
two cases. 

Comment 9

[Navin Gupta]

moving to mozilla 0.9.2. if anyone edits there rules.dat or for some reason 
the rules.dat does not have name for the filter it will crash. 

Comment 10

[Scott MacGregor]

sr=mscott

Comment 11

[Navin Gupta]

hwaara can I get r= ?

Comment 12

[Suresh]

r=suresh

Comment 13

[Christopher Blizzard (:blizzard)]

a=blizzard on behalf of drivers for the trunk

Comment 14

[Navin Gupta]

fix checked in

Comment 15

[Navin Gupta]

brian, please verify this bug.

Comment 16

[Brian Nesse (gone)]

Nope. Now it dies in ParseCondition() probably doing the same m_curFilter stuff 
near the bottom.

Comment 17

[Navin Gupta]

whoops!!!  forgot one more case. patch coming up

Comment 18

[Navin Gupta]

Attachment: test for null before this case also.

Comment 19

[Navin Gupta]

I have tested and it no longer crashes. Sorry, should have been more careful 
the first time. 

Comment 20

[Suresh]

r=suresh

Comment 21

[Scott MacGregor]

sr=mscott

Comment 22

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

That file looks like it needs some major whitespace cleanup... but a=dbaron for
trunk checkin (on behalf of drivers).

Comment 23

[Navin Gupta]

fix checked in. should not crash this time. 

Comment 24

[Brian Nesse (gone)]

This time it works. Crash is gone.

Comment 25

[laurel]

I tried to reproduce a/the crash in this situation with june21 commercial trunk
build, mac OS 9.0 and couldn't.
Marking verified based on my tests and reporter's latest comments.

Comment 26

[laurel]

I tried to reproduce a/the crash in this situation with june21 commercial trunk
build, mac OS 9.0 and couldn't.
Marking verified based on my tests and reporter's latest comments.

Comment 27

[Navin Gupta]

To crash it edit your rules.dat and remove filter name. 

Comment 28

[laurel]

Yes, that's what I did when I tried to reproduce the crash.

Comment 29

[Navin Gupta]

June21 build will have the fix, you need to try a build before 17th.

Comment 30

[laurel]

It's already verified. I'm done.