Skip to content

Commit

Permalink
Explicitly invalidate the global parse hash in SpecParser construct…
Browse files Browse the repository at this point in the history
…or (#409)

Explicitly invalidate the global parse hash in `SpecParser` constructor

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.
Fixes packit/packit-service#2461.

Reviewed-by: Laura Barcziová
  • Loading branch information
2 parents 3fe7b74 + e5d8d48 commit 25735a9
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 25735a9

Please sign in to comment.