wxImage::Scale causes program crash when new size is too large

[Jive Dadson]

I have an app with a "+" button that the user can click on to rescale an image larger. If the user clicks it too many times, the program crashes in wxImage::Scale. The library code does not throw an exception that the application code could handle, nor does it return an image for which IsOk() returns false, either of which would be acceptable if documented. Instead it intentionally crashes the application, even in the Release version.

What happens internally is that wxImage::Scale calls wxImage::ResampleNearest, which attempts to make a new image. ResampleNearest then calls GetData on the new image. GetData then invokes the macro wxCHECK_MSG, which crashes the application. Even if it did not, the next line in ResampleNearest would do so, because it is another call to wxCHECK_MSG.

This is a recurring anti-pattern in image.cpp (at least). Where the function should either throw an exception or return a bad image that will return false for IsOk(), it instead crashes the application on purpose.

I put the following work-around in my code:

{ // RAII block

// Work around bug in wxWidgets. Claim as much space
// as Scale will need. Hope the memory survives long enough
// after we free it on exit from this block.
wxImage new_image;
new_image.Create( new_x, new_y, false );
if(!new_image.IsOk()) {

backout();
return;

}

}

[Jive Dadson]

After studying the code some more, I discover that 1) this a new "feature" and 2) there is a way to override it, namely wxSetAssertHandler. So maybe this should be moved from "defect" to "feature request", or "want to think this over again"? I think it is a defect for sure, but since the behavior is intentional, not all may agree. The default behavior should be either always to throw an exception or always to return quietly with a "bad bit" tucked away in the object. Intentionally crashing an application is a violent act, to be avoided whenever possible! It is the cyanide pill of programming.

The question are these: 1) Can I be sure that all faults will trigger the assert-handler? If so, I can replace it with a handler that throws an exception, and catch them in my code. 2) If I disable the assert-handler, can I be sure that I will always get back a "bad" wxImage or whatever, and never get a SEGV? If so, I can check IsOk(). I am not asking for an answer in these comments, but the answer should be in the documentation.

Most importantly, the default behavior should be changed.

[Jive Dadson]

Okay, one more screed and I will let it go.

I discovered a third way the system can act in an error condition. If the user asks me to open an image file that is corrupted, a dialog pops up. For my program, that is fine. For another it might not be.

I made a mistake in that last comment when I said there were options. There is only one way the system should behave by default, namely the way it did prior to this release. Whether one calls it, "First do no harm," or "Extend, do not change," the principle is not to pull the rug from under those who have working programs to maintain.

[catalin]

First of all I think this is another ticket for which there was no notification sent by trac.

About the ticket - AFAICS the code does return an invalid image when an operation fails. It also asserts which is not at all a "crash".
To disable asserts in release build, you can build wxW with DEBUG_FLAG=0 or see wxDisableAsserts() or the one you already discovered wxSetAssertHandler(NULL).

About comment 2: when loading a corrupt image file you get a log window. To disable it see  wxLogNull.

This is most likely not a bug after all.

[vadz]

Replying to catalin:

First of all I think this is another ticket for which there was no notification sent by trac.

Yes :-( I hope the OP already saw my comment in #14643:2 and changed his login name to avoid this in the future.

About the ticket - AFAICS the code does return an invalid image when an operation fails. It also asserts which is not at all a "crash".

Absolutely.

To disable asserts in release build, you can build wxW with DEBUG_FLAG=0 or see wxDisableAsserts() or the one you already discovered wxSetAssertHandler(NULL).

And neither of those is necessary if you compile your release builds with NDEBUG defined -- as you normally should.

About comment 2: when loading a corrupt image file you get a log window. To disable it see  wxLogNull.

This is most likely not a bug after all.

No, not at all.

To the OP: if you have any questions about this, please post to wx-users.