From 7d37df92ba2a9ab807b1ce408e8a3d6375fa858d Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 7 Nov 2024 10:52:45 -0700 Subject: [PATCH 1/5] Catch (and avoid) infinite recursion in ConfigDict.__getattr__ --- pyomo/common/config.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/pyomo/common/config.py b/pyomo/common/config.py index 651eac35175..fe3bc40aa82 100644 --- a/pyomo/common/config.py +++ b/pyomo/common/config.py @@ -2705,14 +2705,17 @@ def __len__(self): def __iter__(self): return map(attrgetter('_name'), self._data.values()) - def __getattr__(self, name): + def __getattr__(self, attr): # Note: __getattr__ is only called after all "usual" attribute # lookup methods have failed. So, if we get here, we already # know that key is not a __slot__ or a method, etc... - _name = name.replace(' ', '_') - if _name not in self._data: - raise AttributeError("Unknown attribute '%s'" % name) - return ConfigDict.__getitem__(self, _name) + _attr = attr.replace(' ', '_') + # Note: we test for "_data" because finding attributes on a + # partially constructed ConfigDict (before the _data attribute + # was declared) can lead to infinite recursion. + if _attr == "_data" or _attr not in self._data: + raise AttributeError("Unknown attribute '%s'" % attr) + return ConfigDict.__getitem__(self, _attr) def __setattr__(self, name, value): if name in ConfigDict._reserved_words: From 91d74ee7204c735993be1ab78b6f0062f64c9ff2 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 7 Nov 2024 10:53:29 -0700 Subject: [PATCH 2/5] Update AttributeError message to match current Python errors --- pyomo/common/config.py | 4 +++- pyomo/common/tests/test_config.py | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/pyomo/common/config.py b/pyomo/common/config.py index fe3bc40aa82..8465c947a31 100644 --- a/pyomo/common/config.py +++ b/pyomo/common/config.py @@ -2714,7 +2714,9 @@ def __getattr__(self, attr): # partially constructed ConfigDict (before the _data attribute # was declared) can lead to infinite recursion. if _attr == "_data" or _attr not in self._data: - raise AttributeError("Unknown attribute '%s'" % attr) + raise AttributeError( + f"'{type(self).__name__}' object has no attribute '{attr}'" + ) return ConfigDict.__getitem__(self, _attr) def __setattr__(self, name, value): diff --git a/pyomo/common/tests/test_config.py b/pyomo/common/tests/test_config.py index 5d887a9d980..4906382ae78 100644 --- a/pyomo/common/tests/test_config.py +++ b/pyomo/common/tests/test_config.py @@ -2736,7 +2736,9 @@ def test_getattr_setattr(self): ): config.baz = 10 - with self.assertRaisesRegex(AttributeError, "Unknown attribute 'baz'"): + with self.assertRaisesRegex( + AttributeError, "'ConfigDict' object has no attribute 'baz'" + ): a = config.baz def test_nonString_keys(self): From 9b249bfabed3699ea6d498cbd296802226f53515 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 7 Nov 2024 12:48:16 -0700 Subject: [PATCH 3/5] Update initiallization of ConfigValues to be more thread-safe --- pyomo/common/config.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/pyomo/common/config.py b/pyomo/common/config.py index 8465c947a31..fa59d53ead1 100644 --- a/pyomo/common/config.py +++ b/pyomo/common/config.py @@ -1693,17 +1693,23 @@ class UninitializedMixin(object): @property def _data(self): # - # This is a possibly dangerous construct: falling back on - # calling the _default can mask a real problem in the default - # type/value. + # We assume that _default is usually a concrete value. But, we + # also accept a types (classes) and initialization functions as + # defaults, in which case we will construct an instance of that + # class and use that as the default. If they both raise + # exceptions, we will let the original exception propagate up. # try: self._setter(self._default) except: if hasattr(self._default, '__call__'): - self._setter(self._default()) - else: - raise + _default_val = self._default() + try: + self._setter(_default_val) + return self._data + except: + pass + raise return self._data @_data.setter From 23919b164d63678b9546fcb37ac878c4c47bc7f5 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 7 Nov 2024 15:48:25 -0700 Subject: [PATCH 4/5] Disable GC, shorten switchinterval to improve test determinism --- pyomo/common/tests/test_tee.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/pyomo/common/tests/test_tee.py b/pyomo/common/tests/test_tee.py index a5c6ee894b2..2b2a418d63d 100644 --- a/pyomo/common/tests/test_tee.py +++ b/pyomo/common/tests/test_tee.py @@ -10,6 +10,7 @@ # This software is distributed under the 3-clause BSD License. # ___________________________________________________________________________ +import gc import os import time import sys @@ -23,6 +24,20 @@ class TestTeeStream(unittest.TestCase): + def setUp(self): + self.reenable_gc = gc.isenabled() + gc.disable() + # Set a short switch interval so that the threading tests behave + # as expected + self.switchinterval = sys.getswitchinterval() + sys.setswitchinterval(tee._poll_interval / 100) + + def tearDown(self): + sys.setswitchinterval(self.switchinterval) + if self.reenable_gc: + gc.enable() + gc.collect() + def test_stdout(self): a = StringIO() b = StringIO() From 91ec93f77ca62af0ead991f0a9358138c2248850 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Fri, 8 Nov 2024 06:58:19 -0700 Subject: [PATCH 5/5] Add exception testing --- pyomo/common/tests/test_config.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/pyomo/common/tests/test_config.py b/pyomo/common/tests/test_config.py index 4906382ae78..67bb93a7224 100644 --- a/pyomo/common/tests/test_config.py +++ b/pyomo/common/tests/test_config.py @@ -1863,7 +1863,18 @@ def test_default_function(self): c.value() c = ConfigValue('a', domain=int) - with self.assertRaisesRegex(ValueError, 'invalid value for configuration'): + with self.assertRaisesRegex( + ValueError, '(?s)invalid value for configuration.*casting a' + ): + c.value() + + # Test that if both the default and the result from calling the + # default raise exceptions, the propagated exception is from + # castig the original default: + c = ConfigValue(default=lambda: 'a', domain=int) + with self.assertRaisesRegex( + ValueError, "(?s)invalid value for configuration.*lambda" + ): c.value() def test_set_default(self):