Project

General

Profile

Actions

Bug #20307

closed

`Hash#update` from compare_by_identity hash can have unfrozen string keys

Added by nobu (Nobuyoshi Nakada) about 2 months ago. Updated about 1 month ago.

Status:
Closed
Assignee:
-
Target version:
-
[ruby-core:116965]

Description

I don't think this behavior is expected.

i = Hash.new.compare_by_identity
k = "a"
i[k] = 0
h = {}.update(i)
p h.compare_by_identity?  # => false
p h["a"]                  # => 0

k.upcase!
# `k` is still in `h`.
p h.keys.include?(k)      # => true

# but not found.
p((h.fetch(k) rescue $!)) # => #<KeyError: key not found: "A">
h["A"] = 1  
p h                       # => {"A"=>0, "A"=>1}

I expect h to still have "a"=>0 entry.

Actions #1

Updated by nobu (Nobuyoshi Nakada) about 2 months ago

  • Description updated (diff)
Actions #2

Updated by nobu (Nobuyoshi Nakada) about 2 months ago

  • Description updated (diff)
Actions #3

Updated by nobu (Nobuyoshi Nakada) about 2 months ago

  • Description updated (diff)
Actions #4

Updated by nobu (Nobuyoshi Nakada) about 2 months ago

  • Description updated (diff)

Updated by Dan0042 (Daniel DeLorme) about 1 month ago ยท Edited

After the code above, we can do

h.rehash
p h      #=> {"A"=>1}

When a key is clobbered like that due to a rehash, maybe there should be a warning or an error? It seems to me a pretty clear symptom of a bug, either in the ruby code or in the interpreter itself.

Updated by byroot (Jean Boussier) about 1 month ago

I don't understand the bug report nor the patch.

If you make a frozen copy of the provided key, you change the key. e.g.

cache = {}.compare_by_indentity
key = Random.bytes(5)

cache[key] = 1
p cache[key]

I'd expect cache[key] to return 1, with what I understand of your patch, it wouldn't anymore.

Mutable hash keys aren't a new thing, it's only that regular hash have this special behavior for strings to keep a frozen copy of the string key, but they don't do that for any other type.

I don't see why identity hashes would need to copy this behavior.

Updated by byroot (Jean Boussier) about 1 month ago

ignore me, I misunderstood the reproduction script. h is not compared by identity.

Actions #9

Updated by nobu (Nobuyoshi Nakada) about 1 month ago

  • Status changed from Open to Closed

Applied in changeset git|f36a71e26995b69ff72bc132bbcf40ad89571414.


[Bug #20307] Fix Hash#update to make frozen copy of string keys

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0