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

MAYA-113821 MayaUSD : Python bindings un-registration of registered t… #1709

Merged
merged 2 commits into from
Sep 20, 2021

Conversation

boudrey
Copy link
Contributor

@boudrey boudrey commented Sep 17, 2021

…ranslators

Register python to atexit, similar to how it is done for dlls.

@@ -51,12 +51,15 @@ TF_INSTANTIATE_SINGLETON(UsdMayaExportChaserRegistry);

std::map<std::string, UsdMayaExportChaserRegistry::FactoryFn> _factoryRegistry;

bool UsdMayaExportChaserRegistry::RegisterFactory(const std::string& name, FactoryFn fn)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same parameter fromPython added at every Registration to determine if registration is done in c++ (within a .dll) or from Python.

if (fromPython) {
g_pythonUnloaders.emplace_back(func);
if (boost::python::import("atexit")
.attr("register")(&PythonUnload, g_pythonUnloaders.size() - 1)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The magic is here, registering to atexit with static c++ function and g_pythonUnloaders current last element.

@JGamache-autodesk
Copy link
Collaborator

Fixes unregistering all Python entries before the interpreter gets destroyed. The atexit logic should make sure all items registered while Python was live will be run before the interpreter exits, but I wonder if registering for onMayaExit MMessage would be safer.

One interesting question is how does this work if someone unloads the mayaUsdPlugin after an export, then reloads it. Will the callback be correctly unregistered when the module is unloaded, and will they correctly be re-registered when the plugin is reloaded? Is this a case where we should prevent unloading the plugin to preserve the sanity of the registries?

@JGamache-autodesk
Copy link
Collaborator

Seems like having any entry in the registries should disallow unloading the maya plugin.

To test:

  • Create mesh sphere
  • Assign red standard surface
  • loadPlugin mayaUsdPlugin
  • Export all to USD (notice all the registrations with TF_DEBUG=PXRUSDMAYA_REGISTRY)
  • unloadPlugin mayaUsdPlugin
  • loadPlugin mayaUsdPlugin
  • Export all to USD
    BUG: Second export fails with
    Found loaded usdMaya plugin mayaUsd_Translators: UsdMaya/PrimWriter = mesh. No maya plugin. No usdMaya writer plugin for maya type mesh. No maya plugin found. No usdMaya writer plugin for maya type surfaceShape. No maya plugin found. No usdMaya writer plugin for maya type controlPoint. No maya plugin found. No usdMaya writer plugin for maya type deformableShape. No maya plugin found. No usdMaya writer plugin for maya type geometryShape. No maya plugin found. No usdMaya writer plugin for maya type shape. No maya plugin found. No usdMaya writer plugin for maya type dagNode. No maya plugin found. No usdMaya writer plugin for maya type entity. No maya plugin found. No usdMaya writer plugin for maya type containerBase. No maya plugin found.

@boudrey
Copy link
Contributor Author

boudrey commented Sep 17, 2021

This was testing the registration/un-registration of c++ plugin not python. Right? If so, I can create a new JIRA for it and adding that the Python registration/un-registration should also be validated.

As for using onMayaExit MMessage, atexit callback will be called before, which is what I prefer. What do you mean by safer?

@JGamache-autodesk
Copy link
Collaborator

Yes, that test script was for the existing C++ part which is currently broken. The solution will have to be good enough to support Python as well.

I have re-read how atexit works in Python and it is exactly the right function to use. Please disregard the mayaExit comment.

@boudrey
Copy link
Contributor Author

boudrey commented Sep 20, 2021

Logged as MAYA-113849 Failing export after unloading/re-loading mayaUsdPlugin (https://jira.autodesk.com/browse/MAYA-113849)

@boudrey boudrey added the ready-for-merge Development process is finished, PR is ready for merge label Sep 20, 2021
@kxl-adsk kxl-adsk merged commit 9760483 into dev Sep 20, 2021
@kxl-adsk kxl-adsk deleted the boudrey/MAYA-113821/unloaders branch September 20, 2021 13:33
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.

4 participants