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

PureScalaDebugger is free from the bug (as far as tested) #67

Merged
merged 2 commits into from
Dec 1, 2014

Conversation

anatoliykmetyuk
Copy link
Collaborator

No description provided.

AndreVanDelft added a commit that referenced this pull request Dec 1, 2014
PureScalaDebugger is free from the bug (as far as tested)
@AndreVanDelft AndreVanDelft merged commit 5cf03bf into AndreVanDelft:develop Dec 1, 2014
@AndreVanDelft
Copy link
Owner

Does this mean that the debugger problems were due to a Scala Swing bug?
How did you find out?

Is there a reference to that bug?
If so could you include a link in the code?
Else will you report it to the Scala team?

Op 1 dec. 2014, om 18:03 heeft anatoliykmetyuk notifications@github.com het volgende geschreven:

You can merge this Pull Request by running

git pull https://github.com/anatoliykmetyuk/scala develop
Or view, comment on, or merge it at:

#67

Commit Summary

PureScalaDebugger is bug-free (so far)
Exit confirmation dialog added to PureScalaDebugger:
File Changes

M src/subscript-swing/src/swing/PureScalaDebugger.scala (76)
Patch Links:

https://github.com/AndreVanDelft/scala/pull/67.patch
https://github.com/AndreVanDelft/scala/pull/67.diff

Reply to this email directly or view it on GitHub.

@anatoliykmetyuk
Copy link
Collaborator Author

Quite possible it is to the Scala Swing feature (which fails to get fixed properly due to the bug). But I still have doubts.
I don't think it makes any sense to do significant action to fix it, since the readme of the Scala Swing repo says that its development was stopped.

Publishers (https://github.com/scala/scala-swing/blob/master/src/main/scala/scala/swing/Publisher.scala) contain the protected val listeners = new RefSet[Reaction] collection where they store their subscribed reactions. Each reaction gets wrapped into either a weak or a strong reference.

The reference is weak by default, which introduces certain problems (see my large comment in the commit). It is possible to make it strong easily (see my comment).

The problem is that the strong reference is implemented by Scala Swing (same file, StrongReference), not the core Scala. It inherits from scala's native Reference with their custom trait Ref (same file). Only the trait Ref defines the equals and hashCode methods, essential when you are working with a HashSet that backs the entire RefSet. The equals method is defined as follows:

    override def equals(that: Any) = that match {
      case that: ReferenceWrapper[_] =>
        val v1 = this.get
        val v2 = that.get
        v1 == v2
      case _ => false
    }

Which makes it possible to compare ReferenceWrapper's, but not more generic Reference's. WeakReference extends ReferenceWrapper, but StrongReference extends just Reference with Ref. Therefore, it won't make it through the pattern matching and won't equal even to itself. This makes it possible to add a strong reference to the collection of reactions, but makes it impossible to remove it from this collection.

Consider this simple example:

class Proxy[A](val x: A) {
  override def hashCode = x.##
  override def equals(that: Any) = that match {case y: Proxy[A] => x == y.x; case _ => false}
}

scala> val set = mutable.HashSet[Proxy[Int]]()
set: scala.collection.mutable.HashSet[Proxy[Int]] = Set()

scala> set += new Proxy(1)
res18: set.type = Set(Proxy@1)

scala> set -= new Proxy(1)
res19: set.type = Set()

But:

class Proxy[A](val x: A) {
  override def hashCode = x.##
  override def equals(that: Any) = false
}

scala> val set = new mutable.HashSet[Proxy[Int]]
set: scala.collection.mutable.HashSet[Proxy[Int]] = Set()

scala> set += new Proxy(1)
res7: set.type = Set(Proxy@1)

scala> set -= new Proxy(1)
res8: set.type = Set(Proxy@1)

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.

2 participants