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

Added cleanup code such that all code coverage run tests pass. #196

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

ppt-adsk
Copy link
Collaborator

@ppt-adsk ppt-adsk commented Nov 6, 2024

Code coverage builds have to leak retained scene indices because of a crash in the retained scene index destructor, on Clang code coverage builds alone. This creates a maintenance burden when such scene indices also manage other resources. This pull request adds a Destroy() method to certain classes so that proper cleanup can be done in code coverage builds, even if retained scene indices are leaked to avoid crashing.

@ppt-adsk ppt-adsk self-assigned this Nov 6, 2024
_Destroy();
}

void MayaHydraSceneIndex::_Destroy()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Add Destroy() method to perform proper cleanup on code coverage builds.

#ifndef CODE_COVERAGE_WORKAROUND
private:
#endif
void _Destroy();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Add Destroy() method to perform proper cleanup on code coverage builds.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we have a ticket to track when to remove the CODE_COVERAGE_WORKAROUND?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just created one: HYDRA-1297

friend struct PXR_NS::MayaUsdSceneIndexRegistration;
#endif

void Destroy() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Add Destroy() method to perform proper cleanup on code coverage builds.

@@ -339,6 +354,18 @@ struct MayaUsdSceneIndexRegistration : public MayaHydraSceneIndexRegistration
auto proxyShapeSceneIndex = TfDynamic_cast<MayaUsdProxyShapeSceneIndexRefPtr>(pluginSceneIndex);
proxyShapeSceneIndex->UpdateTime();
}

#ifdef CODE_COVERAGE_WORKAROUND
void Destroy() override {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Invoke Destroy() methods.

#ifdef CODE_COVERAGE_WORKAROUND
// Leak the Maya scene index for code coverage, as its base class
// HdRetainedSceneIndex dtor crashes in Windows clang code coverage build.
_mayaHydraSceneIndex->_Destroy();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Invoke Destroy() method.

@ppt-adsk ppt-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge Development process is finished, PR is ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants