413599 - debug build crashes on exit with memory corruption

Status: RESOLVED FIXED

(Toolkit :: Startup and Profile System, defect)

Description

[Aleks Totic]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.11) Gecko/20071127 Firefox/2.0.0.11
Build Identifier: 1/22/2008 trunk

Upon exiting any xulrunner app, windows xulrunner crashes with corrupt heap.
The problem is in nsWindowsWMain.cpp. We clone argv, pass argvClone to main(), then delete argvClone. This crashes, because main() rearranges argv behind our back. I'll attach a sample patch.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Aleks Totic]

Attachment: Sample patch for the nsWindowsWMain.cpp

Comment 2

[timeless]

Comment on attachment 298666 [details] [diff] [review]
Sample patch for the nsWindowsWMain.cpp

check deleteUs for OOM |if (!deleteUs) {
  FreeAllocStrings(argc, argvConverted);
  return 127;
}|

or something...

Comment 3

[Aleks Totic]

Attachment: Revised patch, added OOM check.

Added OOM check per instructions. 

Is checking for OOM on all allocations the offical FF policy? I can't imagine how we would get OOM on startup, but I can understand being extra careful for security reasons.

Comment 4

[Benjamin Smedberg]

Comment on attachment 298783 [details] [diff] [review]
Revised patch, added OOM check.

You don't ever delete[] deleteUs... was that intentional?

Comment 5

[Aleks Totic]

deleteUs gets deleted inside FreeAllocString. That's the way old code worked.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

This patch also fixes the shutdown crash that occurs when running XUL apps with Firefox using the "-app" flag

Comment 7

[Reed Loden [:reed]]

Aleks, why did you reassign this bug to Doug?

Comment 8

[Aleks Totic]

So he can check it in for me. I do not have write privileges (I think). And doug is sitting right next to me.

Comment 9

[Aleks Totic]

Or do you want to check it in? (reed)

Comment 10

[Reed Loden [:reed]]

The assignee is always the person who wrote the patch. This can't be checked-in right now because it does not have approval. Is this needed for beta 3, or can it wait until after the beta 3 freeze is over?

Comment 11

[Doug Turner (:dougt)]

reed, when/if this is approved, please check in.

Comment 12

[Reed Loden [:reed]]

(In reply to comment #11)
> reed, when/if this is approved, please check in.

Will do.

Comment 13

[Reed Loden [:reed]]

Checking in toolkit/xre/nsWindowsWMain.cpp;
/cvsroot/mozilla/toolkit/xre/nsWindowsWMain.cpp,v  <--  nsWindowsWMain.cpp
new revision: 1.4; previous revision: 1.3
done