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

oiiotool: work around static destruction order issue (#3295) #3591

Merged
merged 1 commit into from
Oct 7, 2022
Merged

Conversation

aras-p
Copy link
Contributor

@aras-p aras-p commented Oct 7, 2022

Description

In some cases (at least on Windows, with static linking) the oiiotool main returns zero, but the full process returns 3 error code due to abort() being called. And that is called because a global shared image cache is already destroyed, but the image references from another global (Oiiotool itself) try to use it, get a call into a "pure virtual function" stub that is put by MSVC C runtime into a destroyed object vtable, and that ends up in an abort().

Work around that by making sure various member variables holding ImageRecRef are cleared before main() finishes and global destructors are called.

Tests

N/A -- but it does make running OIIO test suite locally much easier for me. Previously python runners that were launching oiiotool were getting exit code 3 when "everything was fine", so I had to resort to running their final command lines manually. Which is quite cumbersome!

Checklist:

  • I have read the contribution guidelines.
  • If this is more extensive than a small change to existing code, I
    have previously submitted a Contributor License Agreement
    (individual, and if there is any way my
    employers might think my programming belongs to them, then also
    corporate).
  • I have updated the documentation, if applicable.
  • I have ensured that the change is tested somewhere in the testsuite
    (adding new test cases if necessary).
  • My code follows the prevailing code style of this project.

In some cases (at least on Windows, with static linking) the oiiotool
main returns zero, but the full process returns 3 error code due to
abort() being called. And that is called because a global shared
image cache is already destroyed, but the image references from another
global (Oiiotool itself) try to use it, get a call into a "pure virtual
function" stub that is put by MSVC C runtime into a destroyed object
vtable, and that ends up in an abort().

Work around that by making sure various member variables holding
ImageRecRef are cleared before main() finishes and global destructors
are called.
Copy link
Collaborator

@lgritz lgritz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Definitely seems reasonable.

@lgritz lgritz merged commit f60205a into AcademySoftwareFoundation:master Oct 7, 2022
@lgritz
Copy link
Collaborator

lgritz commented Oct 7, 2022

Hmmm, this also makes me think that it probably would have been better for ImageCache::create to have returned a ref-counted pointer. And perhaps for "ot" to be dynamically allocated rather than static, so we can explicitly destroy it before oiiotool exits.

@lgritz
Copy link
Collaborator

lgritz commented Oct 7, 2022

Does this patch fully address #3295 ?

lgritz pushed a commit to lgritz/OpenImageIO that referenced this pull request Oct 7, 2022
…Foundation#3295) (AcademySoftwareFoundation#3591)

In some cases (at least on Windows, with static linking) the oiiotool
main returns zero, but the full process returns 3 error code due to
abort() being called. And that is called because a global shared
image cache is already destroyed, but the image references from another
global (Oiiotool itself) try to use it, get a call into a "pure virtual
function" stub that is put by MSVC C runtime into a destroyed object
vtable, and that ends up in an abort().

Work around that by making sure various member variables holding
ImageRecRef are cleared before main() finishes and global destructors
are called.
@aras-p
Copy link
Contributor Author

aras-p commented Oct 7, 2022

Does this patch fully address #3295 ?

It does for me, but I dunno if it does for the original bug reporter. My guess would be on "yes", from their posted callstack.

@aras-p aras-p deleted the exit-order branch October 7, 2022 19:56
lgritz pushed a commit to lgritz/OpenImageIO that referenced this pull request Oct 8, 2022
…Foundation#3295) (AcademySoftwareFoundation#3591)

In some cases (at least on Windows, with static linking) the oiiotool
main returns zero, but the full process returns 3 error code due to
abort() being called. And that is called because a global shared
image cache is already destroyed, but the image references from another
global (Oiiotool itself) try to use it, get a call into a "pure virtual
function" stub that is put by MSVC C runtime into a destroyed object
vtable, and that ends up in an abort().

Work around that by making sure various member variables holding
ImageRecRef are cleared before main() finishes and global destructors
are called.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants