Skip to content

_util: make DictMixin unhashable to fix hash/equality contract violation - #716

Merged
phw merged 1 commit into
quodlibet:mainfrom
gaoflow:fix/dictmixin-unhashable
Jun 28, 2026
Merged

_util: make DictMixin unhashable to fix hash/equality contract violation#716
phw merged 1 commit into
quodlibet:mainfrom
gaoflow:fix/dictmixin-unhashable

Conversation

@gaoflow

@gaoflow gaoflow commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Fixes #603.

Problem

DictMixin.__eq__ compares objects by their tag content:

def __eq__(self, other):
    return dict(self.items()) == other

But __hash__ was set to object.__hash__, which is identity-based. This violates Python's fundamental requirement that objects which compare equal must have the same hash value:

from mutagen.flac import FLAC
f1 = FLAC("test.flac")
f2 = FLAC("test.flac")
assert f1 == f2          # True — same tags
assert hash(f1) == hash(f2)  # False — broken!

Any code that puts FLAC (or other tag) objects into sets, uses them as dict keys, or relies on the hash/equality contract is silently broken.

Fix

Set __hash__ = None in DictMixin, making instances explicitly unhashable — consistent with Python's own mutable dict. All DictMixin subclasses (tag types, file types) are mutable mappings, so unhashability is the correct behavior.

Testing

Added a regression test in TDictMixin.test_unhashable. Full suite (3993 tests) passes.


This pull request was prepared with the assistance of AI, under my direction and review.

DictMixin.__eq__ compares objects by their content (tag key-value pairs),
but __hash__ was set to object.__hash__ (identity-based). This violates
Python's requirement that equal objects have equal hash values: two tag
objects loaded from the same file compare equal yet produce different
hashes, silently breaking set membership, dict keys, and any code that
relies on the hash/equality invariant.

Fix by setting __hash__ = None, making DictMixin instances explicitly
unhashable - consistent with Python's built-in mutable dict.

Fixes quodlibet#603
@phw
phw requested review from lazka and phw June 27, 2026 17:53

@phw phw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Right. the DictMixin must not be hashable. The docs say explicitly:

If a class defines mutable objects and implements an eq() method, it should not implement hash(), since the implementation of hashable collections requires that a key’s hash value is immutable (if the object’s hash value changes, it will be in the wrong hash bucket).

There is some small potential of this breaking some code, though, if code relied on the hasability of the instances. But regardless, I think this should be fixed and we mention this change prominently in the changelog.

@lazka

lazka commented Jun 27, 2026

Copy link
Copy Markdown
Member

risky, but makes sense.

There are more places with the same pattern

@phw
phw merged commit e89c77a into quodlibet:main Jun 28, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FLAC objects that compare equal do not have the same hash value

3 participants