In wrapSchemaBase, replace the __getattribute__ method as was already…

… being

done in wrapObject. This overloaded __getattribute__ actually allows any of
the UsdSchemaBase APIs to be called (as all of the methods at this level can
still return useful information). But its purpose is to block calls to APIs
defined on subclasses.

Provided similar protection for UsdGeomPrimvar, which is an odd case that is
like a schema class, but operates on a single attribute and so is not derived
from SchemaBase.

Added some test cases, including an explicit test of issue PixarAnimationStudios#872, a crash when
calling UsdGeom.Xformable().ClearXformOpOrder(). TestUsdRiSplineAPI had to be
updated because a test in there was not expecting the RuntimeException from
an invalid object.

pxr/usd/lib/usd/CMakeLists.txt

@@ -189,6 +189,7 @@ pxr_test_scripts(
     testenv/testUsdPrims.py
     testenv/testUsdReferences.py
     testenv/testUsdRelationships.py
+    testenv/testUsdSchemaBasePy.py
     testenv/testUsdSchemaRegistry.py
     testenv/testUsdSpecializes.py
     testenv/testUsdStage.py
@@ -830,6 +831,12 @@ pxr_register_test(testUsdSchemaBase
     EXPECTED_RETURN_CODE 0
 )
 
+pxr_register_test(testUsdSchemaBasePy
+    PYTHON
+    COMMAND "${CMAKE_INSTALL_PREFIX}/tests/testUsdSchemaBasePy"
+    EXPECTED_RETURN_CODE 0
+)
+
 pxr_register_test(testUsdSchemaRegistry
     PYTHON
     COMMAND "${CMAKE_INSTALL_PREFIX}/tests/testUsdSchemaRegistry"

pxr/usd/lib/usd/testenv/testUsdSchemaBasePy.py

@@ -0,0 +1,40 @@
+#!/pxrpythonsubst
+#
+# Copyright 2017 Pixar
+#
+# Licensed under the Apache License, Version 2.0 (the "Apache License")
+# with the following modification; you may not use this file except in
+# compliance with the Apache License and the following modification to it:
+# Section 6. Trademarks. is deleted and replaced with:
+#
+# 6. Trademarks. This License does not grant permission to use the trade
+#    names, trademarks, service marks, or product names of the Licensor
+#    and its affiliates, except as required to comply with Section 4(c) of
+#    the License and to reproduce the content of the NOTICE file.
+#
+# You may obtain a copy of the Apache License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the Apache License with the above modification is
+# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the Apache License for the specific
+# language governing permissions and limitations under the Apache License.
+
+from pxr import Gf, Tf, Sdf, Usd, Vt
+import unittest, math
+
+class TestUsdSchemaBase(unittest.TestCase):
+
+    def test_InvalidSchemaBase(self):
+        sb = Usd.SchemaBase()
+        # It should still be safe to get the prim, but the prim will be invalid.
+        p = sb.GetPrim()
+        with self.assertRaises(RuntimeError):
+            p.IsActive()
+        # It should still be safe to get the path, but the path will be empty.
+        self.assertEqual(sb.GetPath(), Sdf.Path())
+
+if __name__ == "__main__":
+    unittest.main()

pxr/usd/lib/usd/wrapSchemaBase.cpp

@@ -39,6 +39,44 @@ using namespace boost::python;
 
 PXR_NAMESPACE_USING_DIRECTIVE
 
+// We override __getattribute__ for UsdSchemaBase to check object validity
+// and raise an exception instead of crashing from Python.
+
+// Store the original __getattribute__ so we can dispatch to it after verifying
+// validity.
+static TfStaticData<TfPyObjWrapper> _object__getattribute__;
+
+// This function gets wrapped as __getattribute__ on UsdSchemaBase.
+static object
+__getattribute__(object selfObj, const char *name) {
+    // Allow attribute lookups if the attribute name starts with '__', or
+    // if the object's prim is valid. Also add explicit exceptions for every
+    // method on this base class. The real purpose here is to protect against
+    // invalid calls in subclasses which will try to actually manipulate the
+    // underlying (invalid) prim and likely crash.
+    if ((name[0] == '_' && name[1] == '_') ||
+        extract<UsdSchemaBase &>(selfObj)().GetPrim().IsValid() ||
+        strcmp(name, "GetPrim") == 0 ||
+        strcmp(name, "GetPath") == 0 ||
+        strcmp(name, "GetSchemaClassPrimDefinition") == 0 ||
+        strcmp(name, "GetSchemaAttributeNames") == 0 ||
+        strcmp(name, "GetSchemaType") == 0 ||
+        strcmp(name, "IsAPISchema") == 0 ||
+        strcmp(name, "IsConcrete") == 0 ||
+        strcmp(name, "IsTyped") == 0 ||
+        strcmp(name, "IsAppliedAPISchema") == 0 ||
+        strcmp(name, "IsMultipleApplyAPISchema") == 0) {
+        // Dispatch to object's __getattribute__.
+        return (*_object__getattribute__)(selfObj, name);
+    } else {
+        // Otherwise raise a runtime error.
+        TfPyThrowRuntimeError(
+            TfStringPrintf("Accessed schema on invalid prim"));
+    }
+    // Unreachable.
+    return object();
+}
+
 void wrapUsdSchemaBase()
 {
     class_<UsdSchemaBase> cls("SchemaBase");
@@ -68,4 +106,8 @@ void wrapUsdSchemaBase()
         .def(!self)
 
         ;
+
+    // Save existing __getattribute__ and replace.
+    *_object__getattribute__ = object(cls.attr("__getattribute__"));
+    cls.def("__getattribute__", __getattribute__);
 }

pxr/usd/lib/usdGeom/testenv/testUsdGeomPrimvar.py

@@ -505,6 +505,12 @@ def test_PrimvarInheritance(self):
         self.assertEqual(s4p.FindPrimvarWithInheritance(UsdGeom.Tokens.primvarsDisplayColor).GetAttr().GetPrim(),
                 s1.GetPrim())
 
+    def test_InvalidPrimvar(self):
+        p = UsdGeom.Primvar()
+        # This used to crash before the Primvar __getattribute__ method
+        # was overridden to test the validity of the underlying prim.
+        with self.assertRaises(RuntimeError):
+            p.BlockIndices()
 
 if __name__ == "__main__":
     unittest.main()

pxr/usd/lib/usdGeom/testenv/testUsdGeomXformable.py

@@ -660,5 +660,18 @@ def test_Bug109853(self):
         # warning.
         x.GetLocalTransformation(Usd.TimeCode.Default())
 
+    def test_InvalidXformable(self):
+        xf = UsdGeom.Xformable()
+        # This used to crash before the SchemaBase __getattribute__ method
+        # was overridden to test the validity of the underlying prim.
+        with self.assertRaises(RuntimeError):
+            xf.ClearXformOpOrder()
+        # It should still be safe to get the prim, but the prim will be invalid.
+        p = xf.GetPrim()
+        with self.assertRaises(RuntimeError):
+            p.IsActive()
+        # It should still be safe to get the path, but the path will be empty.
+        self.assertEqual(xf.GetPath(), Sdf.Path())
+
 if __name__ == "__main__":
     unittest.main()

pxr/usd/lib/usdGeom/wrapPrimvar.cpp

@@ -102,14 +102,40 @@ _GetTimeSamplesInInterval(const UsdGeomPrimvar &self,
 
 static size_t __hash__(const UsdGeomPrimvar &self) { return hash_value(self); }
 
+// We override __getattribute__ for UsdGeomPrimvar to check object validity
+// and raise an exception instead of crashing from Python.
+
+// Store the original __getattribute__ so we can dispatch to it after verifying
+// validity.
+static TfStaticData<TfPyObjWrapper> _object__getattribute__;
+
+// This function gets wrapped as __getattribute__ on UsdGeomPrimvar.
+static object
+__getattribute__(object selfObj, const char *name) {
+    // Allow attribute lookups if the attribute name starts with '__', or
+    // if the object's prim is valid.
+    if ((name[0] == '_' && name[1] == '_') ||
+        extract<UsdGeomPrimvar &>(selfObj)().GetAttr().GetPrim().IsValid() ||
+        strcmp(name, "GetAttr") == 0) {
+        // Dispatch to object's __getattribute__.
+        return (*_object__getattribute__)(selfObj, name);
+    } else {
+        // Otherwise raise a runtime error.
+        TfPyThrowRuntimeError(
+            TfStringPrintf("Accessed invalid attribute as a primvar"));
+    }
+    // Unreachable.
+    return object();
+}
 
 } // anonymous namespace 
 
 void wrapUsdGeomPrimvar()
 {
     typedef UsdGeomPrimvar Primvar;
 
-    class_<Primvar>("Primvar")
+    class_<Primvar> clsObj("Primvar");
+    clsObj
         .def(init<UsdAttribute>(arg("attr")))
 
         .def(self == self)
@@ -180,5 +206,9 @@ void wrapUsdGeomPrimvar()
     to_python_converter<std::vector<UsdGeomPrimvar>,
                         TfPySequenceToPython<std::vector<UsdGeomPrimvar>>>();
     implicitly_convertible<Primvar, UsdAttribute>();
+
+    // Save existing __getattribute__ and replace.
+    *_object__getattribute__ = object(clsObj.attr("__getattribute__"));
+    clsObj.def("__getattribute__", __getattribute__);
 }
 

pxr/usd/lib/usdRi/testenv/testUsdRiSplineAPI.py

@@ -40,7 +40,8 @@ def test_Basic(self):
 
         # can't use these if not properly initialized
         bogusSpline = UsdRi.SplineAPI()
-        assert not bogusSpline.Validate()[0]
+        with self.assertRaises(RuntimeError):
+            bogusSpline.Validate()[0]
 
         light = UsdLux.SphereLight.Define(stage, '/Light')
         rod = UsdRi.PxrRodLightFilter.Define(stage, '/Light/Rod')