Skip to content
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

Jump to bottom

AnyRefMap.getOrElseUpdate is faulty #8213

Closed
scabug opened this issue Jan 30, 2014 · 5 comments
Closed

AnyRefMap.getOrElseUpdate is faulty #8213

scabug opened this issue Jan 30, 2014 · 5 comments
Assignees
@Ichoran
Labels
blocker
Milestone
2.11.0-RC1

Comments

@scabug
Copy link

scabug commented Jan 30, 2014

The dotty compiler crashed with an "ArrayIndexOutofBounds" exception in "getOrElseUpdate" on an AnyRefMap. I could not minimize it, but think I know where the failure comes from. Here's the code of getOrElseUpdate:

  override def getOrElseUpdate(key: K, defaultValue: => V): V = {
    val h = hashOf(key)
    val i = seekEntryOrOpen(h, key)
    if (i < 0) {
      val value = defaultValue
      _size += 1
      val j = i & IndexMask
      _hashes(j) = h
      _keys(j) = key.asInstanceOf[AnyRef]
      _values(j) = value.asInstanceOf[AnyRef]
      if ((i & VacantBit) != 0) _vacant -= 1
      else if (imbalanced) repack()
      value
    }
    else _values(i).asInstanceOf[V]
  }

The error happens if the defaultValue computation causes a size increase and rehash of the map. In that case the value of i seems to be no longer valid.
The actual call causing the error was:

    typedTree.getOrElseUpdate(expanded(tree), typer.typed(tree, pt))

Indeed the default value expression typer.typed(tree, pt) could easily create enough new typedTree entries to cause a rehash.
@scabug
Copy link
Author

scabug commented Jan 30, 2014

Imported From: https://issues.scala-lang.org/browse/SI-8213?orig=1
Reporter: @odersky
Affected Versions: 2.11.0-M7

@scabug
Copy link
Author

scabug commented Jan 30, 2014

@Ichoran said (edited on Jan 30, 2014 6:58:17 PM UTC):
I hadn't considered that defaultValue would alter the map itself. Easy fix, though.

@scabug
Copy link
Author

scabug commented Jan 30, 2014

@Ichoran said:
scala/scala#3434

@scabug scabug closed this as completed Jan 30, 2014
@scabug scabug added the blocker label Apr 7, 2017
@scabug scabug added this to the 2.11.0-RC1 milestone Apr 7, 2017
@scabug scabug mentioned this issue Apr 7, 2017
Closed
@Ichoran Ichoran mentioned this issue Sep 4, 2018
Merged
@SethTisue
Copy link
Member

SethTisue commented Sep 4, 2018 •
edited

fwiw, I think this could plausibly have simply been closed as "wontfix". blowing up if you change the map out from under itself like this doesn't seem like a bug to me, necessarily. /cc @hrhino

@Ichoran
Copy link

Ichoran commented Sep 4, 2018

One could wontfix it, but it has the unfriendly property then that m.getOrElseUpdate(k, f()) is not the same as the apparently logically equivalent if (!(m contains k)) m(k) = f(); m(k). Having to maintain the mental distinction is an unwelcome burden on the user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Ichoran Ichoran

Labels
blocker
Projects
None yet
Milestone
2.11.0-RC1
Development

No branches or pull requests
3 participants
@SethTisue @Ichoran @scabug