472097 - xul!nsMemoryImpl::IsLowMemory is broken on systems w/ lots of vm

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[timeless]

http://msdn.microsoft.com/en-us/library/aa366772(VS.85).aspx

On computers with more than 4 GB of memory, the MEMORYSTATUS structure can return incorrect information, reporting a value of –1 to indicate an overflow. If your application is at risk for this behavior, use the GlobalMemoryStatusEx function instead of the GlobalMemoryStatus function.

Minefield on my laptop was reporting it was out of memory. Which was odd because i had between 200 and 400mb of ram available.

The code is here:

>  196:     *result = ((float)stat.dwAvailPageFile / stat.dwTotalPageFile) < 0.1;
xul!nsMemoryImpl::IsLowMemory+0x10:
0:000> dt stat
Local var @ 0x12ca00 Type _MEMORYSTATUS
   +0x000 dwLength         : 0x20
   +0x004 dwMemoryLoad     : 0x4f
   +0x008 dwTotalPhys      : 0x5ff5d000
   +0x00c dwAvailPhys      : 0x13cd8000
   +0x010 dwTotalPageFile  : 0xffffffff
   +0x014 dwAvailPageFile  : 0x10299000
   +0x018 dwTotalVirtual   : 0x7ffe0000
   +0x01c dwAvailVirtual   : 0x78ada000
0:000> dt stat _MEMORYSTATUS dwAvailPageFile
Local var @ 0x12ca00 Type _MEMORYSTATUS
   +0x014 dwAvailPageFile : 0x10299000
0:000> dt stat _MEMORYSTATUS dwTotalPageFile
Local var @ 0x12ca00 Type _MEMORYSTATUS
   +0x010 dwTotalPageFile : 0xffffffff
0:000> ? 0x10299000 / 0xffffffff
Evaluate expression: 0 = 00000000

We're using the wrong function to manage memory.

FWIW, I've seen this dialog a lot on my laptop and never understood it.
I have 1.5g of ram and a variable sized swap file.

01/01/09  08:56 AM     3,221,225,472 pagefile.sys
               1 File(s)  3,221,225,472 bytes
               0 Dir(s)   7,714,263,040 bytes free

1.5+3.2 > 4.0, which means we hit the caveat described in the tech note.

Comment 1

[timeless]

Attachment: this should work

i can't test this, because atm |make| is crashing on my computer :)

note that i'm not sure the logic is particularly great.

if available page file is 1g and total page file is 11g, then simply fixing the code to use the wider api, we'd still say "low memory".

Comment 2

[Aiko]

Comment on attachment 355379 [details] [diff] [review]
this should work

>+    MEMORYSTATUSEX stat;
>+    GlobalMemoryStatusEx(&stat);

The documentation of MEMORYSTATUSEX  states:

dwLength 
The size of the structure, in bytes. You must set this member before calling GlobalMemoryStatusEx.

Comment 3

[timeless]

Attachment: fill in size [Checkin: Comment 5 & 9]

Comment 4

[timeless]

Attachment: one unfortunate result of this check being broken

Comment 5

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/mozilla-central/rev/7eb0af556933

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 355394 [details] [diff] [review]
fill in size
[Checkin: Comment 5 & 9]

a1912=beltzner

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 355394 [details] [diff] [review]
fill in size
[Checkin: Comment 5 & 9]

a1912=beltzner

Comment 9

[Serge Gautherie (:sgautherie)]

Comment on attachment 355394 [details] [diff] [review]
fill in size
[Checkin: Comment 5 & 9]


http://hg.mozilla.org/releases/mozilla-1.9.1/rev/ee795036176e

Comment 10

[u279076]

What is the best/simplest way for QA to verify this bug?