pxr.UsdGeom.Xformable.ClearXformOpOrder on an invalid prim crashes

[marktucker]

Description of Issue

Invoking ClearXformOpOrder on an invalid UsdGeom.Xformable object in python will cause a crash.

Steps to Reproduce

1. start python
2. run:
   from pxr import UsdGeom
   p = UsdGeom.Xformable()
   p.ClearXformOpOrder()
   The process will crash.

There are about a dozen levels in the call stack where the proverbial null pointer check could be added, but I'm not sure which of those is the right level, in line with existing protections for python functions on invalid Usd objects (trading off between the number of functions to which a null pointer check must be added vs the additional runtime of performing redundant null pointer checks).

Package Versions

19.05

[spiffmon]

Thanks for reporting, @marktucker ! I think the desired fix here is to add similar getattribute treatment as is given in usd/wrapObject.cpp to wrapSchemaBase.cpp ... and also the one or two "property schemas" we have, like UsdGeomPrimvar.

[marktucker]

I just did a quick test, and was able to implement this for UsdGeomXformable and UsdGeomPrimvar pretty easily... If you want to tell me which other classes fall into this category I can wrap them all into one PR? I also didn't whitelist any particular functions since I didn't see any of them making sense to call on "null" prims. Let me know if you think there are functions that should pass through the getattribute filter.

[spiffmon]

Hi @marktucker ! If you hit wrapSchemaBase.cpp in core usd, that should handle all derived generated schema classes. UsdGeomPrimvar is the only "USD Object wrapper" schema I can recall that needs special treatment of its own.

I do think we want to pass at least GetPrim(), GetPath(), and the operator bool (though I'm not sure how to indicated that in this context.. my python-wrapping fu is not great). @gitamohr might have suggestions of other methods it would be good to pass.

[marktucker]

Not claiming the above pull request is complete, but I think it's easier to have something concrete to look at when discussing possible refinements. I ended up allowing all SchemaBase methods to be called on invalid SchemaBase objects, because they are safe, and make sense even if the underlying prim is null. But this change blocks all methods of all subclasses.

[jilliene]

Filed as internal issue #USD-5373

[sunyab]

Closing this issue out as it was addressed by #876. I also wanted to note we recently added similar protection to UsdGeom.XformOp in commit 91f37aa. Thanks1