Skip to content

Fix RuntimeError caused by analyzing live objects with __getattribute__ or descriptors #2687

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

Merged
merged 3 commits into from
Feb 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ Release date: TBA

Closes pylint-dev/pylint#10112

* Fix crash when `sys.modules` contains lazy loader objects during checking.

Closes #2686
Closes pylint-dev/pylint#8589


What's New in astroid 3.3.8?
Expand Down
13 changes: 10 additions & 3 deletions astroid/nodes/node_classes.py
Original file line number Diff line number Diff line change
Expand Up @@ -5514,6 +5514,7 @@ def _create_basic_elements(
"""Create a list of nodes to function as the elements of a new node."""
elements: list[NodeNG] = []
for element in value:
# NOTE: avoid accessing any attributes of element in the loop.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's remove this comment and the one below. If the implementation of const_factory ever changes this is outdated. Imo, comments should be at the offending code (which you already did πŸ˜„)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure I understand.
The problem happens in the loop, so possibly it could occur directly in _create_basic_elements() without need for const_factory() to cause it.
I added the extra comments because the value is dangerous for the whole loop body (including const_factory()), until the value might be identified as const type.

element_node = const_factory(element)
element_node.parent = node
elements.append(element_node)
Expand All @@ -5526,6 +5527,7 @@ def _create_dict_items(
"""Create a list of node pairs to function as the items of a new dict node."""
elements: list[tuple[SuccessfulInferenceResult, SuccessfulInferenceResult]] = []
for key, value in values.items():
# NOTE: avoid accessing any attributes of both key and value in the loop.
key_node = const_factory(key)
key_node.parent = node
value_node = const_factory(value)
Expand All @@ -5536,18 +5538,23 @@ def _create_dict_items(

def const_factory(value: Any) -> ConstFactoryResult:
"""Return an astroid node for a python value."""
assert not isinstance(value, NodeNG)
# NOTE: avoid accessing any attributes of value until it is known that value
# is of a const type, to avoid possibly triggering code for a live object.
# Accesses include value.__class__ and isinstance(value, ...), but not type(value).
# See: https://github.com/pylint-dev/astroid/issues/2686
value_type = type(value)
assert not issubclass(value_type, NodeNG)

# This only handles instances of the CONST types. Any
# subclasses get inferred as EmptyNode.
# TODO: See if we should revisit these with the normal builder.
if value.__class__ not in CONST_CLS:
if value_type not in CONST_CLS:
node = EmptyNode()
node.object = value
return node

instance: List | Set | Tuple | Dict
initializer_cls = CONST_CLS[value.__class__]
initializer_cls = CONST_CLS[value_type]
if issubclass(initializer_cls, (List, Set, Tuple)):
instance = initializer_cls(
lineno=None,
Expand Down
9 changes: 9 additions & 0 deletions tests/test_raw_building.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import pytest

import tests.testdata.python3.data.fake_module_with_broken_getattr as fm_getattr
import tests.testdata.python3.data.fake_module_with_collection_getattribute as fm_collection
import tests.testdata.python3.data.fake_module_with_warnings as fm
from astroid.builder import AstroidBuilder
from astroid.const import IS_PYPY, PY312_PLUS
Expand Down Expand Up @@ -121,6 +122,14 @@ def test_module_object_with_broken_getattr(self) -> None:
# This should not raise an exception
AstroidBuilder(AstroidManager()).inspect_build(fm_getattr, "test")

def test_module_collection_with_object_getattribute(self) -> None:
# Tests https://github.com/pylint-dev/astroid/issues/2686
# When astroid live inspection of module's collection raises
# error when element __getattribute__ causes collection to change size.

# This should not raise an exception
AstroidBuilder(AstroidManager()).inspect_build(fm_collection, "test")


@pytest.mark.skipif(
"posix" not in sys.builtin_module_names, reason="Platform doesn't support posix"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class Changer:
def __getattribute__(self, name):
list_collection.append(self)
set_collection.add(self)
dict_collection[self] = self
return object.__getattribute__(self, name)


list_collection = [Changer()]
set_collection = {Changer()}
dict_collection = {Changer(): Changer()}
Loading