#1022
- Created on2011-07-13 18:59
- Reported by
- StatusFixed
- PriorityNormal (3)
- Assigned to
- Milestone
- Plan Level-
- Due Date-
Race Problem and Liveness Problem: akka.util.Switch
The Akka Switch has room for some improvements.
-------------------------------------------------------
1) All operations that do a change on the, do this using a synchronized block. So you have a synchronized block on the outside and an AtomicBoolean on the inside. This is a bit overkill. Solution: change the type of Switch to boolean var and to make sure you can do cheap reads in a concurrent environment, make it volatile.
-------------------------------------------------------
2) It can happen that an operation is executed on a switch, while it is not in the status expected. This is caused by a non atomic check/do operation. E.g.
def ifOffYield[T](action: ⇒ T): Option[T] = {
if (!switch.get) Some(action)
else None
}
In this case it can happen that the check is done to make sure it is off, the thread sees it is off, and continues to the next step. But before that step begins, a different thread turns the switch on. The consequence is that the operation is executed while the switch is on, instead of off. Depending on the semantics of this class (which are not available in the documentation) this problem can be classified as a data race.
Solutions:
- Either add it in the documentation that it is a best effort.
- Wrap synchronized blocks on almost all but the simplest getters to guarantee a complete serialization.
-------------------------------------------------------
3) There are some methods that allow functions to be executed inside a synchronized block. This can be quite dangerous because you don't have control on how long the lock will be held (so other contending for the same lock could start to block as well). And this is a recipe for deadlocks. For more information see chapter 10.1.4. Open Calls of Java Concurrency in Practice. Small quote: "calling an alien method with a lock held is difficult to analyze and therefore risky."
My suggestion would be to provide methods that use a timeout which lowers the chance of deadlocks. The methods without a timeout could use some kind of akka default for the switch timeout, and if explicitly provided, a user can make it own choice if he want to wait for ever.. or not..
-------------------------------------------------------
1) All operations that do a change on the, do this using a synchronized block. So you have a synchronized block on the outside and an AtomicBoolean on the inside. This is a bit overkill. Solution: change the type of Switch to boolean var and to make sure you can do cheap reads in a concurrent environment, make it volatile.
-------------------------------------------------------
2) It can happen that an operation is executed on a switch, while it is not in the status expected. This is caused by a non atomic check/do operation. E.g.
def ifOffYield[T](action: ⇒ T): Option[T] = {
if (!switch.get) Some(action)
else None
}
In this case it can happen that the check is done to make sure it is off, the thread sees it is off, and continues to the next step. But before that step begins, a different thread turns the switch on. The consequence is that the operation is executed while the switch is on, instead of off. Depending on the semantics of this class (which are not available in the documentation) this problem can be classified as a data race.
Solutions:
- Either add it in the documentation that it is a best effort.
- Wrap synchronized blocks on almost all but the simplest getters to guarantee a complete serialization.
-------------------------------------------------------
3) There are some methods that allow functions to be executed inside a synchronized block. This can be quite dangerous because you don't have control on how long the lock will be held (so other contending for the same lock could start to block as well). And this is a recipe for deadlocks. For more information see chapter 10.1.4. Open Calls of Java Concurrency in Practice. Small quote: "calling an alien method with a lock held is difficult to analyze and therefore risky."
My suggestion would be to provide methods that use a timeout which lowers the chance of deadlocks. The methods without a timeout could use some kind of akka default for the switch timeout, and if explicitly provided, a user can make it own choice if he want to wait for ever.. or not..
Leave a comment
I've ScalaDocced the entire class, pushing this back to 2.0 as a feature request (redesign to minimize/remove needed locking etc)
on 2011-07-15 09:54 *
By viktorklang
Assigned to set to viktorklang
Status changed from New to Fixed
Updating tickets (#967, #974, #975, #976, #980, #981, #989, #990, #992, #993, #994, #999, #1000, #1004, #1008, #1011, #1015, #1018, #1022, #1023, #1024, #1025, #1027, #1028, #1029, #1030, #1032, #1033, #1036, #1047, #1053, #1062, #1067, #1068, #1069, #1072, #1075, #1078, #1082, #1102, #1107, #1110, #1111, #1115, #1116, #1121, #1122, #1123, #1124)