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

RESOLVED FIXED in mozilla1.9.2a1

Status

()

RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: timeless, Assigned: timeless)

Tracking

Trunk
mozilla1.9.2a1
x86
Windows XP
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(status1.9.1 .2-fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

10 years ago
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.
(Assignee)

Comment 1

10 years ago
Created attachment 355379 [details] [diff] [review]
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".
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #355379 - Flags: review?
(Assignee)

Updated

10 years ago
Attachment #355379 - Flags: review? → review?(benjamin)

Comment 2

10 years ago
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.
(Assignee)

Comment 3

10 years ago
Created attachment 355394 [details] [diff] [review]
fill in size
[Checkin: Comment 5 & 9]
Attachment #355379 - Attachment is obsolete: true
Attachment #355394 - Flags: review?(benjamin)
Attachment #355379 - Flags: review?(benjamin)
(Assignee)

Comment 4

10 years ago
Created attachment 355395 [details]
one unfortunate result of this check being broken
QA Contact: xbl → xpcom

Updated

10 years ago
Attachment #355394 - Flags: review?(benjamin) → review+
http://hg.mozilla.org/mozilla-central/rev/7eb0af556933
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
(Assignee)

Updated

9 years ago
Duplicate of this bug: 490163
(Assignee)

Updated

9 years ago
Attachment #355394 - Flags: approval1.9.1.2?
Comment on attachment 355394 [details] [diff] [review]
fill in size
[Checkin: Comment 5 & 9]

a1912=beltzner
Attachment #355394 - Flags: approval1.9.1.2? → approval1.9.1.2+
Comment on attachment 355394 [details] [diff] [review]
fill in size
[Checkin: Comment 5 & 9]

a1912=beltzner
Keywords: checkin-needed
Whiteboard: [needs 1.9.1.x landing]
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
Attachment #355394 - Attachment description: fill in size → fill in size [Checkin: Comment 5 & 9]
status1.9.1: --- → .2-fixed
Keywords: checkin-needed
Whiteboard: [needs 1.9.1.x landing]
What is the best/simplest way for QA to verify this bug?
(Assignee)

Updated

9 years ago
Duplicate of this bug: 507754
You need to log in before you can comment on or make changes to this bug.