Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libjxl uses abort() #1450

Open
jpcha2 opened this issue May 26, 2022 · 1 comment
Open

libjxl uses abort() #1450

jpcha2 opened this issue May 26, 2022 · 1 comment
Assignees
Labels

Comments

@jpcha2
Copy link

jpcha2 commented May 26, 2022

Describe the bug
libjxl calls abort(). Since it is intended as a library to be used in other projects, it shouldn't do this.

To Reproduce
Steps to reproduce the behavior:
Basically any flow, including the loading of a corrupted file or API misuse, that results in a call to one of the following:

  • jxl::Abort()
  • JxlThreadParallelRunner::Abort()

Expected behavior
Calling abort() within a library is a bug because the library has no way of knowing what the calling application is doing. If the calling application is a process that is either statically linked to the library, a process that is dynamically linked to it or a process that calls a DLL that loads the library in any fashion, that abort() call will lead to the entire application suddenly crashing, sometimes for apparently no reason or a very trivial one.

Consider, for instance, a case where libjxl is being used to store and load small images used as, say, employee mugshots in a listing in an app that does something completely unrelated to graphics. The mugshots are an unimportant feature and an invalid thumbnail file should ideally just result in a blank square, but the current implementation will crash the entire app as a result of the abort() call. The HR app for payroll crashing because the new intern's photo was causing one of the many image-file loading libraries to intentionally abort() the program cannot be described as a good design.

In my view, there is no error that can be encountered within a library that justifies aborting the calling process.

There are two ways to address this:

  • Use the approach taken in butteraugli.cc, which is to wrap the fault-handler that calls abort() in an "enable_checks" switch
  • Implement a proper fault-handling scheme that results in the relevant error being sent back to the calling application via the return value of the currently executing libjxl function.

Screenshots
n/a

Environment

  • OS: Windows
  • Compiler version: MSVC 2017 (ver 14.16)]
  • CPU type: x86_64
  • cjxl/djxl version string: n/a

Additional context

@rickbrew
Copy link

rickbrew commented Sep 6, 2022

I second this. I'd love to use libjxl to add JPEG XL support to Paint.NET by way of including @0xC0000054 's plugin, but I can't let a failure in loading a file cause the app to crash and lose all of the user's work and data. abort() is a banned method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants