Skip to content

Commit

Permalink
fix: Fix testrender GPU regression with bad destruction order (Academ…
Browse files Browse the repository at this point in the history
…ySoftwareFoundation#1814)

A few months back, PR AcademySoftwareFoundation#1733 seems to have switched the order that
testrender destroys the shading system versus the renderer (services).

This made some subtle bugs that were only symptomatic for GPU renders,
but it's because of the destructor order, where the shadingsystem's
dtr still references the renderer, which cannot be destroyed yet.

The clue is that the SS's constructor takes the RS pointer as an
argument. The RS, then, must have been constructed before the SS, and
therefore we should expect it to be a requirement for the RS to
outlast the lifetime of the SS. (Complex objects should be destroyed
in the opposite order that they were constructed, if they contain
references to each other.)

One code change is needed to avoid the sanitizer errors that the
incorrect change was originally meant to address: clear shaders when
SimpleRayTracer clears.

Signed-off-by: Larry Gritz <[email protected]>

---------

Signed-off-by: Larry Gritz <[email protected]>
  • Loading branch information
lgritz authored May 8, 2024
1 parent bba9b35 commit f6b88b9
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/testrender/optixraytracer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1253,7 +1253,7 @@ OptixRaytracer::finalize_pixel_buffer()
void
OptixRaytracer::clear()
{
shaders().clear();
SimpleRaytracer::clear();
OPTIX_CHECK(optixDeviceContextDestroy(m_optix_ctx));
m_optix_ctx = 0;
}
Expand Down
6 changes: 6 additions & 0 deletions src/testrender/simpleraytracer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1101,4 +1101,10 @@ SimpleRaytracer::render(int xres, int yres)



void
SimpleRaytracer::clear()
{
shaders().clear();
}

OSL_NAMESPACE_EXIT
2 changes: 1 addition & 1 deletion src/testrender/simpleraytracer.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class SimpleRaytracer : public RendererServices {
virtual void prepare_render();
virtual void warmup() {}
virtual void render(int xres, int yres);
virtual void clear() {}
virtual void clear();

// After render, get the pixels into pixelbuf, if they aren't already.
virtual void finalize_pixel_buffer() {}
Expand Down
2 changes: 1 addition & 1 deletion src/testrender/testrender.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ main(int argc, const char* argv[])

// We're done with the shading system now, destroy it
rend->clear();
delete rend;
delete shadingsys;
delete rend;
return EXIT_SUCCESS;
}

0 comments on commit f6b88b9

Please sign in to comment.