Fixed race condition in RecycleBin. #1828
Conversation
| _size: Int | ||
| count ::= this _list count | ||
| isFull ::= this count == this _size | ||
| isEmpty ::= this _list empty | ||
| init: func (=_size, =_free) { this _list = SynchronizedList<T> new(_size) } | ||
| _mutex := Mutex new() |
There was a problem hiding this comment.
the count, isFull and isEmpty is not thread safe, change to private?
There was a problem hiding this comment.
On second thought, this class should be able to use without a mutex, then those properties should be public anyway.
emilwestergren
force-pushed
the
recyclebin_thread_safe_fix
branch
2 times, most recently
from
9106672
to
20aa5eb
Compare
emilwestergren
force-pushed
the
recyclebin_thread_safe_fix
branch
from
20aa5eb
to
d79664f
Compare
| result: T = null | ||
| (index > -1) ? this _list remove(index) : result | ||
| this _mutex lock() |
There was a problem hiding this comment.
@sebastianbaginski Somewhere you changed to put result: T = null inside the lock, is that good as a general rule or was it a special case? I don't remember the specifics.
There was a problem hiding this comment.
It was here #1821 (comment)
Yes, it does matter for generics, because this line result: T = null can trigger the "constructor" of the Pointer_Class due to lazy initialization. This constructor can then modify a static bool flag.
There was a problem hiding this comment.
Thanks. Looks like it's being fixed, but won't make it to us any time soon I'm afraid. ooc-lang/rock#1004
In the mean time, @emilwestergren - move line 35 below line 36.
|
This removes the only use of |
|
I vote for keeping it. |
|
Updated |
Replaced
SynchronizedListwithVectorList+Mutexto maintain thread safety.@erikhagglund peer review