From 0824c3e1b6671d641d495ff736d83dd8bd53ad96 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Sat, 15 Aug 2020 19:05:06 +0000 Subject: [PATCH 1/3] Prevent Aborted (core dumped) in tf.nest.assert_same_structure for type mismatch in passed args This PR tries to address the issue raised in 42329 where tf.nest.assert_same_structure will abort with core dump when check_types is passed with a non-bool value. The issue is related to pybind where type mismatch will throw out an error and further cause 'pybind11::error_already_set' error. This PR explicitly convert check_types to bool before passing to pybind, and, in case check_types is not bool, an ValueError will be thrown out gracefully by python itself. This will be better than process abort (core dump). This PR fixes 42329. Signed-off-by: Yong Tang --- tensorflow/python/util/nest.py | 3 +++ tensorflow/python/util/nest_test.py | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/tensorflow/python/util/nest.py b/tensorflow/python/util/nest.py index 66f43a3d682..0807674e9f0 100644 --- a/tensorflow/python/util/nest.py +++ b/tensorflow/python/util/nest.py @@ -392,6 +392,9 @@ def assert_same_structure(nest1, nest2, check_types=True, TypeError: If the two structures differ in the type of sequence in any of their substructures. Only possible if `check_types` is `True`. """ + # Convert to bool explicitly as otherwise pybind will not be able# to handle + # type mismatch message correctly. See GitHub issue 42329 for details. + check_types = bool(check_types) try: _pywrap_utils.AssertSameStructure(nest1, nest2, check_types, expand_composites) diff --git a/tensorflow/python/util/nest_test.py b/tensorflow/python/util/nest_test.py index fb3f2102ba7..bb9530aa57d 100644 --- a/tensorflow/python/util/nest_test.py +++ b/tensorflow/python/util/nest_test.py @@ -1218,6 +1218,12 @@ class NestTest(parameterized.TestCase, test.TestCase): expected, ) + def testInvalidCheckTypes(self): + with self.assertRaises(ValueError): + nest.assert_same_structure( + nest1=array_ops.zeros((1)), nest2=array_ops.ones((1,1,1)), + check_types=array_ops.ones((2))) + class NestBenchmark(test.Benchmark): From 7170d8e8468568ae100e925741e59b7270b009ce Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Sat, 15 Aug 2020 19:12:31 +0000 Subject: [PATCH 2/3] Convert expand_composites to bool explicitly to prevent pybind caused process abort See GitHub issue 42329 for details. Also fixes the pylint failure Signed-off-by: Yong Tang --- tensorflow/python/util/nest.py | 1 + tensorflow/python/util/nest_test.py | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/tensorflow/python/util/nest.py b/tensorflow/python/util/nest.py index 0807674e9f0..e072bebe6f2 100644 --- a/tensorflow/python/util/nest.py +++ b/tensorflow/python/util/nest.py @@ -395,6 +395,7 @@ def assert_same_structure(nest1, nest2, check_types=True, # Convert to bool explicitly as otherwise pybind will not be able# to handle # type mismatch message correctly. See GitHub issue 42329 for details. check_types = bool(check_types) + expand_composites = bool(expand_composites) try: _pywrap_utils.AssertSameStructure(nest1, nest2, check_types, expand_composites) diff --git a/tensorflow/python/util/nest_test.py b/tensorflow/python/util/nest_test.py index bb9530aa57d..31030d0117b 100644 --- a/tensorflow/python/util/nest_test.py +++ b/tensorflow/python/util/nest_test.py @@ -1221,8 +1221,12 @@ class NestTest(parameterized.TestCase, test.TestCase): def testInvalidCheckTypes(self): with self.assertRaises(ValueError): nest.assert_same_structure( - nest1=array_ops.zeros((1)), nest2=array_ops.ones((1,1,1)), + nest1=array_ops.zeros((1)), nest2=array_ops.ones((1, 1, 1)), check_types=array_ops.ones((2))) + with self.assertRaises(ValueError): + nest.assert_same_structure( + nest1=array_ops.zeros((1)), nest2=array_ops.ones((1, 1, 1)), + expand_composites=array_ops.ones((2))) class NestBenchmark(test.Benchmark): From 70a041db8aa09ff6a59219c75c01ded9bfe8e8dd Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Wed, 19 Aug 2020 20:02:32 +0000 Subject: [PATCH 3/3] Expand captured exception to ValueError+TypeError, for internal test fix Signed-off-by: Yong Tang --- tensorflow/python/util/nest_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tensorflow/python/util/nest_test.py b/tensorflow/python/util/nest_test.py index 31030d0117b..768a2440d61 100644 --- a/tensorflow/python/util/nest_test.py +++ b/tensorflow/python/util/nest_test.py @@ -1219,11 +1219,11 @@ class NestTest(parameterized.TestCase, test.TestCase): ) def testInvalidCheckTypes(self): - with self.assertRaises(ValueError): + with self.assertRaises((ValueError, TypeError)): nest.assert_same_structure( nest1=array_ops.zeros((1)), nest2=array_ops.ones((1, 1, 1)), check_types=array_ops.ones((2))) - with self.assertRaises(ValueError): + with self.assertRaises((ValueError, TypeError)): nest.assert_same_structure( nest1=array_ops.zeros((1)), nest2=array_ops.ones((1, 1, 1)), expand_composites=array_ops.ones((2)))