out of memory bug on win32 platforms

9 views
Skip to first unread message

Jelle Geerts

unread,
Aug 23, 2008, 3:01:19 PM8/23/08
to vim...@vim.org
Hello,

When allocating memory, lalloc() is called, which uses mch_avail_mem(). But
mch_avail_mem() sometimes fails. It returns the available physical memory +
available page file memory, which can sometimes wrap around 32-bits.

Also, mch_avail_mem() uses GlobalMemoryStatus() which isn't working properly on
computers with more than 4 GiB of memory installed. There is an extension
available on NT platforms 0x0500, called GlobalMemoryStatusEx(). As opposed to
GlobalMemoryStatus(), the extension works reliably with systems that have more
than 4 GiB of memory installed.

Please see the attachment for the diff file for my patch. If the comments I
have added are too wordy or someone does not like them, just remove them ;)

Thanks,
Jelle Geerts

os_win32.c.diff

George V. Reilly

unread,
Aug 23, 2008, 11:00:46 PM8/23/08
to vim...@googlegroups.com
2008/8/23 Jelle Geerts <jelle...@gmail.com>:

It looks to me like your patch will fail if ms.dwAvailPhys is more
than 4GiB, due to underflow of (0xffffff00 - ms.dwAvailPhys). This is
a DWORDLONG, which is always an unsigned 64-bit int.

Also, shouldn't that be msex.ullAvailPhys, not ms.dwAvailPhys in the
(_WIN32_WINNT >= 0x0500) && !defined(_WIN64) case?

Better, I think, to return (long_u) min(0xFFFFFF00, msex.ullAvailPhys
+ msex.ullAvailPageFile). Or for Bram to widen this signature to a
64-bit int for all platforms. Linux and Mac boxes with >4GiB are going
to be common soon.
--
/George V. Reilly geo...@reilly.org
http://www.georgevreilly.com/blog http://blogs.cozi.com/tech

Jelle Geerts

unread,
Aug 24, 2008, 2:16:10 PM8/24/08
to vim...@googlegroups.com
Hey,

You were right and wrong. Using min() wouldn't help since we pass the
overflowed result of the addition, which could be say 1, even when there is
_much_ more memory available on the computer. It could happen on 64-bit systems
too when the user has a lot of memory (say 64 GiB, we don't see these today a
lot, if at all).

I've prepared a new version of the patch, hopefully it is good enough.
It seems that on Mac and Unix systems, the function mch_avail_mem() isn't used
or even defined at all, so this patch is only needed for Windows at the time.

--
Jelle Geerts

Jelle Geerts

unread,
Aug 24, 2008, 2:16:32 PM8/24/08
to vim...@googlegroups.com
And heres the patch I forgot to attach.

--
Jelle Geerts

os_win32.c.diff

Bram Moolenaar

unread,
Aug 24, 2008, 10:28:14 PM8/24/08
to Jelle Geerts, vim...@vim.org

Jelle Geerts wrote:

Thanks for looking into this. The comment is confusing: returning
non-zero for failure. The result is always non-zero.

I build one binary for all systems. So the choice for the function to
call must be done at runtime, not a build time. There are a few
examples of using the system version. You will need to find out on what
systems GlobalMemoryStatusEx() is available.

Looking at this code I wonder why we add the pagefile space. The whole
idea of this mch_avail_mem() is to find out how much fits in memory, so
that we can decide when to start using the swap file.

--
I think that you'll agree that engineers are very effective in their social
interactions. It's the "normal" people who are nuts.
(Scott Adams - The Dilbert principle)

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ download, build and distribute -- http://www.A-A-P.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

Bram Moolenaar

unread,
Aug 24, 2008, 10:28:14 PM8/24/08
to George V. Reilly, vim...@googlegroups.com

George Reilly wrote:

I don't think that Vim needs to know if you have more than 4 Gbyte
available. Just make sure mch_avail_mem() doesn't wrap around and
returns a number close to 4 Gbyte when there is more.

Unfortunately, 64-bit support isn't very well standardized. And there
are systems that don't support it at all.

--
A radioactive cat has eighteen half-lives.

Reply all
Reply to author
Forward
0 new messages