add Mutex.Locked() ?
Petar Maymounkov
sync.Mutex.Locked() bool
returning true if the Mutex is currently locked.
Why? It is for debug purposes.
When writing complex code, I often end up in a situation
where some methods of a complex class are meant to be called only from
inside
a Lock(), obtained in the caller (whereas other methods don't need a
lock).
In complex function call-patterns it is hard to verify that you've
locked everywhere before calling one of those functions that require
a lock.
It would be an easy debug check to add in each function that
expects a lock, like this:
func (o *Object) method() {
if !o.mutex.Locked() {
panic()
}
...
}
Are people happy with this?
--P
Dmitriy Vyukov
What about mutex.assertLocked()? It simplifies user code and
emphasizes that the function is only for debug purposes and the return
value should not be acted upon.
Petar Maymounkov
bflm
the most important objective to me is - it would have a race, so it's
not really usable. Imagine some other goroutine has unlocked the mutex
object just after Locked() returned true. What's the returned true
value then good for?
Yves Junqueira
might release or get the lock immediately after your "if" block.
And Dmitriy's assertLocked() would panic? That's very destructive for
a debugging method, no?
If a project is complex enough that it's hard to keep track of
resource locks, a review of the synchronization model would be a good
idea. Personally I'd just use channels.
--
Yves Junqueira <http://cetico.org/about>
Petar Maymounkov
> The result Locked() would not be meaningful, since another participant
> might release or get the lock immediately after your "if" block.
This will produce false positives, that's fine for debugging purposes.
>
> And Dmitriy's assertLocked() would panic? That's very destructive for
> a debugging method, no?
That's a matter of taste. Forgetting to lock is more disruptive, I would argue.
Strange behavior without a panic might sink your 10s to 100s of hours
to track down. I'd rather it explode (even in the hands of the user).
>
> If a project is complex enough that it's hard to keep track of
> resource locks, a review of the synchronization model would be a good
> idea. Personally I'd just use channels.
That's not the rule, in my book. If locking is simple (i.e. there are
no multiple
interacting locks, rather just a single one), there is no need for channels.
This is witnesses by a lot of the code in the Go Library.
Rob 'Commander' Pike
There is more merit in a boolean-valued function that returns whether the lock could be grabbed. If the return value is true, the lock is now held; if false, it's not. That's safe, but still unwelcome. I am very reluctant to start adding all the niggly little lock features people are used to from languages where locks are all they have.
-rob
Steven
Yes, but with channels, implementing your feature is trivial. A unit
buffer channel, you can select on it with a default. If you acquire
the lock (receive a value), it wasn't locked.
Brad Fitzpatrick
If you want it for debugging, put it in when you're debugging. It's easy to do. Just don't make it part of the standard package, because it will be used by people who are not debugging and their code will break.
There is more merit in a boolean-valued function that returns whether the lock could be grabbed. If the return value is true, the lock is now held; if false, it's not. That's safe, but still unwelcome. I am very reluctant to start adding all the niggly little lock features people are used to from languages where locks are all they have.
Dmitriy Vyukov
> The result Locked() would not be meaningful, since another participant
> might release or get the lock immediately after your "if" block.
It returns true iff the mutex is locked by the *current* goroutine. So
there is no races.
Perhaps a better name would be assertHeld() or assertAcquired().
> And Dmitriy's assertLocked() would panic? That's very destructive for
> a debugging method, no?
Just like out of bounds accesses or null pointer dereferences. There
is not much you can do in such situations other than crash.
Jessta
> On Sun, May 22, 2011 at 11:21 PM, Yves Junqueira <yv...@cetico.org> wrote:
>> The result Locked() would not be meaningful, since another participant
>> might release or get the lock immediately after your "if" block.
>
>
> It returns true iff the mutex is locked by the *current* goroutine. So
> there is no races.
> Perhaps a better name would be assertHeld() or assertAcquired().
Mutexes on Go don't work like that. Goroutines don't have identities
so there is no way to tell which goroutine did what.
"current goroutine" has no meaning in the spec.
- jessta
--
=====================
http://jessta.id.au
Rob 'Commander' Pike
That's not true. "Current goroutine" is the one executing the code in question. It's never an ambiguous concept.
Which is not to offer support for the proposal, just to correct your statement.
-rob
Jessta
> That's not true. "Current goroutine" is the one executing the code in question. It's never an ambiguous concept.
Good point, but what I meant was that the spec doesn't allow the code
being executed to have any way of identifying which goroutine it's
executing in. So I can't write code that will execute differently
depending on what goroutine it's executing in.
> Which is not to offer support for the proposal, just to correct your statement.
--
=====================
http://jessta.id.au
Dmitriy Vyukov
> On Mon, May 23, 2011 at 7:44 PM, Rob 'Commander' Pike <r...@google.com> wrote:
>> That's not true. "Current goroutine" is the one executing the code in question. It's never an ambiguous concept.
>
> Good point, but what I meant was that the spec doesn't allow the code
> being executed to have any way of identifying which goroutine it's
> executing in. So I can't write code that will execute differently
> depending on what goroutine it's executing in.
Yeah, if the function returns true/false then it allows one to build
goroutine-local storage on top of it. Suppose you have 2 goroutines
and 1 mutex, first goroutine locks the mutex for whole lifetime, then
isLocked() can be used to subscribe 2 element array :)
Russ Cox
$ godoc sync Mutex
...
A locked Mutex is not associated with a particular goroutine.
It is allowed for one goroutine to lock a Mutex and then
arrange for another goroutine to unlock it.
jp
>
> I'm very much against IsLocked() bool or IsUnlocked() bool, but I wouldn't
_mind_ seeing:
>
> func (m *Mutex) AssertLocked() // panics if not locked, no return value
> func (m *Mutex) TryLock() bool // grabs lock and returns true if not already
locked
>
> ... but any time I've wanted either, I eventually realized my locking was
complex enough that I should be using channels instead.
>
>
I am for "func (m *Mutex) TryLock() bool".
boost (C++ library) has TryLock() while IsLocked() had been removed after
discussion.
A practical example.
It is implementation of Future's with mutex.
You can compare it with one on channels and goroutines:
http://beyondcpp.com/post/4746885487/futures-in-go#notes
I'd say, Future with channels was complex enough that I should be using mutex
instead.
===============
type Future struct {
val interface {}
mu sync.Mutex
}
func NewFuture() (self *Future) {
self = new(Future)
self.mu.Lock()
return
}
func (self *Future) Set(val interface{}) {
self.mu.TryLock() // second call to Set() will panic in Unlock() without it
self.val = val
self.mu.Unlock()
}
func (self *Future) Get() (ret interface{}) {
self.mu.Lock()
ret = self.val
self.mu.Unlock()
return
}
===============
Kyle Lemons
j...@webmaster.ms
Being set once it can be read many times.
Reads performed after the future is set should not block.
You can have a look how far went an attempt of implementing future as
a
chan reader end:
http://beyondcpp.com/post/4746885487/futures-in-go#notes
Web
Reads performed after the future is set should not block.
You can have a look how far went an attempt of implemented future as a
chan reader end:
http://beyondcpp.com/post/4746885487/futures-in-go#notes
Kyle Lemons
Future differs from chan a little bit.Being set once it can be read many times.
Reads performed after the future is set should not block.
j...@webmaster.ms
printnl(future.Get())
printnl(future.Get()) // <-- it must not block
Please have a look: http://beyondcpp.com/post/4746885487/futures-in-go
There is already the code you just post, and probably, your next tries
as well.
Last try (which solves all issues) is rather complex.
Kyle Lemons
go func() { time.Sleep(1e9); future.Set(35) }()
printnl(future.Get())
printnl(future.Get()) // <-- it must not block
Please have a look: http://beyondcpp.com/post/4746885487/futures-in-go
There is already the code you just post, and probably, your next tries
as well.
Last try (which solves all issues) is rather complex.
> What's wrong with:
>
> type FutureInt chan int
> func NewFutureInt() FutureInt { return make(chan int, 1) }
> func (f FutureInt) Get() int {
> val := <-f
> f <- val
> return val
>
> }
>
> func (f FutureInt) Set(val int) {
> f <- val
Rob 'Commander' Pike
-rob
j...@webmaster.ms
> > > func (f FutureInt) Get() int {
> > > val := <-f
> > > f <- val
>
> Notice that it's sent BACK on the channel to make it callable multiple
> times. The only blocking on subsequent GETs should be if they're called
> concurrently.
So, reading from a channel with 1 element buffer and then sending the
same value back works pretty much like mutex.Lock() and mutex.Unlock()
> This is wrong if Set can be called more than once; that's about the only
> issue I can see with this implementation.
called second time.
if(TryLock()) {...} allows to choose what happens on the second Set()
- either panic or overwriting the value.
Channel version can try to read the channel to do that.
Thus mutexes made on top of channels with 1 element buffer do have its
TryLock().
Russ Cox
> self.mu.TryLock() // second call to Set() will panic in Unlock() without it
> self.val = val
> self.mu.Unlock()
> }
This is not a compelling argument for TryLock.
If multiple calls to Set are not allowed, then the TryLock
line has no effect and can be deleted. If multiple calls
to Set are allowed, then there is a race here between
Set and Get, so that the lock is no longer protecting
anything: a Set that starts while a Get is in progress
will blithely skip the TryLock (the lock is held by Get)
and overwrite val while Get is reading it. Then both
Get and Set will unlock the lock, which will (one hopes)
cause a panic.
This is exactly the reason that we don't have TryLock,
and incidentally also why the memory model is so weak.
To do otherwise encourages people to be clever
(which usually leads to being wrong) instead of stopping
at the simple, obviously correct solution.
Russ
jimr
Steven Blenkinsop
I very much like the "self-pumping" channel solutiondiscussed in the 2nd article.
j...@webmaster.ms
stop with the reinventing the wheel ?
On Aug 31, 2:37 am, Steven Blenkinsop <steven...@gmail.com> wrote:
David Symonds
> Would it make sense to add the futures (and MVar) to std library to
> stop with the reinventing the wheel ?
This is a wheel that few people are using.