Skip to content

Commit

Permalink
Explicitly invalidate the global parse hash in SpecParser constructor
Browse files Browse the repository at this point in the history
Python can reuse id of objects after they are garbage-collected, however
the global parse hash contains an id of `SpecParser` instance in order
to force parsing on context switches, and when a different instance has
the same id, it has no way to detect that.

Explicitly invalidate the global parse hash when a `SpecParser` instance
is created to prevent this issue.

Signed-off-by: Nikola Forró <[email protected]>
  • Loading branch information
nforro committed Sep 4, 2024
1 parent 3fe7b74 commit e5d8d48
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 2 deletions.
11 changes: 9 additions & 2 deletions specfile/spec_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ def __init__(
self.force_parse = force_parse
self.spec = None
self.tainted = False
# explicitly invalidate the global parse hash, this `SpecParser` instance could have
# been assigned the same id as a previously deleted one and parsing could be
# improperly skipped
SpecParser._last_parse_hash = None

def __eq__(self, other: object) -> bool:
if not isinstance(other, SpecParser):
Expand All @@ -71,9 +75,12 @@ def __eq__(self, other: object) -> bool:
def __repr__(self) -> str:
return f"SpecParser({self.sourcedir!r}, {self.macros!r}, {self.force_parse!r})"

def id(self) -> int:
return id(self)

def __deepcopy__(self, memo: Dict[int, Any]) -> "SpecParser":
result = self.__class__.__new__(self.__class__)
memo[id(self)] = result
memo[self.id()] = result
for k, v in self.__dict__.items():
if k in ["spec", "tainted"]:
continue
Expand Down Expand Up @@ -359,7 +366,7 @@ def parse(
"""
# calculate hash of all input parameters
payload = (
id(self),
self.id(),
self.sourcedir,
self.macros,
self.force_parse,
Expand Down
10 changes: 10 additions & 0 deletions tests/integration/test_specfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,16 @@ def test_parse_if_necessary(spec_macros):
flexmock(SpecParser).should_call("_do_parse").once()
assert spec1.expanded_name == "test"
assert spec1.expanded_version == "28.1.2~rc2"
flexmock(SpecParser).should_receive("id").and_return(12345)
flexmock(SpecParser).should_call("_do_parse").once()
spec = Specfile(spec_macros)
flexmock(SpecParser).should_call("_do_parse").never()
assert spec.expanded_name == "test"
spec = None
flexmock(SpecParser).should_call("_do_parse").once()
spec = Specfile(spec_macros)
flexmock(SpecParser).should_call("_do_parse").never()
assert spec.expanded_name == "test"


@pytest.mark.skipif(
Expand Down

0 comments on commit e5d8d48

Please sign in to comment.