290284 - [gfx,widget,windows] Use std::nothrow when using new[]

Status: RESOLVED WONTFIX

(Core :: Graphics, defect)

Description

[ArronM (:paper)]

MSVC7 terminates mozilla.exe when an OOM error occurs, whereas MSVC6 just
returns null.  While all the official mozilla.org releases are built with MSVC6,
some of the projects that use Gecko build their releases with MSVC7.

This bug/patch will add (std::nothrow) to some of the new[] calls in the Windows
code, specifically gfx and widget.

Comment 1

[ArronM (:paper)]

Attachment: Add std::nothrow to new[] calls

I probably missed some, but I think I got the 'important' ones.

I'm assuming that MSVC7 returns null when there's an OOM and a (std:nothrow). 
That appears to be the standard thing to do, but standards and Microsoft,
well.. I guess if someone who has MSVC7 wants to check, it would be good to
know.

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

does MSVC6 support std::nothrow?

Comment 3

[ArronM (:paper)]

(In reply to comment #2)
> does MSVC6 support std::nothrow?

It either supports it or ignores it.  The results are the same, a null is
returned on OOM.  I'm using MSVC6.

Comment 4

[ArronM (:paper)]

Ignore Comment #3, I don't know what I'm talking about.  Reassigning to default.
 Someone else will have to handle this.

Comment 5

[ArronM (:paper)]

Moments after removing myself from the bug, biesi helped me figure stuff out.  I
actually did know what I was talking about in Comment #3.  Reassigning..

Comment 6

[Bernard Alleysson]

(In reply to comment #1)
I've tried with MSVC 7.1 (.NET 2003): new returns null with std::nothrow.

See http://msdn.microsoft.com/msdnmag/issues/03/09/LegacySTLFix/default.aspx:
With MSVC 7.1 new throws by default when <new> is included and returns null when
<new.h> is included (I've tried). MS did that on purpose and it's my favorite
solution for Windows/MSVC 7.1 (when you don't use the STL), there's no need to
add (std::nothrow) everywhere.

HTH

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

> there's no need to add (std::nothrow) everywhere.

There is, because there are other compilers than MSVC. Additionally, that
behaviour may change in future versions. Indeed, from your url: "Note that in
Visual C++ 2005, new always throws unless you explicitly link to nothrownew.obj."

Comment 8

[Bernard Alleysson]

At least maybe a macro like
"#define NEW new (std::nothrow)" for MSVC7 and "#define NEW new" for everything
else would be better, it could be defined in some central place so that it could
be extended to cover other compilers used now (mingw, gcc, ...) and in the future .

(In reply to comment #7)
I agree. Changing the allocator behaviour at compiler/linker level might also be
a problem with external C++ source libraries (for instance myspell, I don't know
whether myspell needs std::nothrow or not). 

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 180685 [details] [diff] [review]
Add std::nothrow to new[] calls

ok, so, this should be ok then since on windows those are the only compilers we
support.

we should probably use this for non-array new too...

r=biesi

Comment 10

[timeless]

we really need macros to be used for the whole tree, at least one for
constructors and one for new, they should share the configure test with
CPP_THROW_NEW (see bug 149032).

iirc the fun bits come when you deal with placement new.

note that this isn't a new problem (ok, it's a problem with new, but it dates to
2003...),
http://www.pseudorandom.org/irclog/mozilla/%23mozilla/%23mozilla.2003-03/%23mozilla.2003-03-16.log

forcing NEW_H back to <new.h> sounds like a much better approach (i think i
complained about this in the NEW_H bug, but i'm not certain).

Comment 11

[ArronM (:paper)]

I'm not sure if I have my syntax right, but something like 

if test "$_MSC_VER" -ge "1300"; then NEW_H=new.h; fi

on line 7584ish of configure might work, but probably not standard.  Someone
else would have to make the patch

Comment 12

[Peter Weilbacher]

Any final solution probably depends on what will finally be decided for bug 353144.

Comment 13

[Ginn Chen]

Any update for this bug?

I just found some "new char" in mozilla-central/netwerk/streamconv/converters.
And it caused a crash when memory run out.

Comment 14

[timeless]

ginn: file a new bug. this bug is for gfx,widget,windows as is clearly indicated in the summary (and worse, it's in Core Graveyard, how could you expect it to go anywhere).

Comment 15

[Benoit Girard (:BenWa)]

I think this is no longer applicable with Infallible memory allocation:
https://developer.mozilla.org/en/Infallible_memory_allocation

Please re-open if I'm mistaking.