ILock, Disposable or Not?

[Troy Howard]

All,

Sergey has finished up the interfaces for the lucere.io.concurrency
namespace. One of the core interfaces, ILock, seems like it could
benefit from implementing IDisposable.

I'd like to open the discussion on that specific refactoring point.
Does this sound like a good idea? Can anyone envision any problems
with this?

My perspective is:

A lock seems like something that should be scoped very tightly.
Implementing IDisposable will allow me to define a critical region by
scoping a lock within a 'using' block. This is similar to the 'lock'
blocks we already have for multi-threading (aka 'synchronized' in
Java). It also means that if there is code written that is not careful
about releasing locks in a timely fashion, the GC will collect and
dispose of those locks for us when it's determined they've fallen out
of scope.

This all seems like an improvement over the current structure.

Thanks,
Troy

[Gokhan Demir]

+1

Hi,
I think implementing IDisposable also make the code much more
cleaner.
If I didn't remember wrong, castle guys goes with the same idea :
https://github.com/castleproject/Castle.Core/blob/master/src/Castle.Core/Core/Internal/ILockHolder.cs

Gokhan Demir.

[Ciaran Roarty]

Troy

Just to be clear, you mean the using construct will call Dispose rather than
the GC will collect?

If it manages an unmanaged resource then it is a no-brainer to make it
IDisposable, IMO.

Ciaran

All,

My perspective is:

Thanks,
Troy

--
You received this message because you are subscribed to the Google Groups
"Lucere Development" group.
To post to this group, send email to lucer...@googlegroups.com.
To unsubscribe from this group, send email to
lucere-dev+...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/lucere-dev?hl=en.

[Hakeem]

So If I understood it correctly, code like below

object _syncObj = new object();

void SomeMethod()
{
lock(_syncObj)
{
//critical section
}
}

If this is what we want to have, what is the Dispose method disposing.
The lock gets released as soon as the thread exits the critical
region. One exception I can think of is when an exception is thrown by
the code inside the protected block. In that case the thread aborts
and its data becomes eligible for GC. Again, I might not have
understood the intent

Unrelated, but the best way to have as few locks as possible is to
make all member fields immutable and have as little state as possible
in the objects

Thanks!

[Troy Howard]

Yes. 'Using' will handle it in normal scenarios and GC will be a fallback.

The Castle code is interesting because the don't implement disposable in the Lock class, but instead on a wrapper LockHandle. Is this for ref counting?

Thanks,
Troy  

[Troy Howard]

No I meant...

public void SpecialTask()
{
using(var lock = LockFactory.MakeLock("foo"))
{
// do something that requires a lock..
}
}

and the comment about GC is that if someone implemented this without using:

public void SpecialTask()
{
var lock = LockFactory.MakeLock("foo");

// do something that requires a lock..
}

and failed to call Dispose on the lock instance, when the method
completed, the GC would detect that it had fallen out of scope, and
call Dispose for us. It's a nice fallback mechanism.

Thanks,
Troy

[Sergey Mirvoda]

Hello All!

Sure it should be IDisposable.

Look at LockFactory With abstract static class from Lucene.

And it counterpart With static method from Lucere's LockFactory.

One issue with IDisposable.

Lock intended to throw LockReleaseFailedException when it unable to free lock.

Using implemented in .NET in this way

var lock = LockFactory.MakeLock("foo")

try

{

    /do work

}

finally

{

 lock.Dispose()

}

now you will get exception from Finally block.

Even worse.

If you will implement IDisposable as recommended (with Finalizer)

you throw exception from Finilizer and in a  fatal and unpredicted way.

--
--Regards, Sergey Mirvoda