I have a class that is used to acquire and release locks for files. I use a customKey class that is just a ReentrantReadWriteLock with an id string (id being the file). For some reason, this only works in some situations, and in most it hangs on unlock of all things - my debugger traces it's use all the way there and then just gets stuck.
What am I doing wrong? I would get if a thread crashed and did not release it's lock but here a thread tries to call unlock and does not go any further.
This is the method for locking:
override fun acquire(lockId: String?, ownerId: String?, sequence: Long): Boolean
{
if (lockId != null)
{
lockedList.find { customLock -> customLock.Id == lockId }.apply {
if (this != null) //lock already exists for this ID
{
println("Locking file $lockId Existing lock")
this.writeLock().lock()
println("Locked file $lockId")
} else //lock does not exist
{
val newLock = CustomLock(lockId)
lockedList.add(newLock)
println("Locking file $lockId")
newLock.writeLock().lock()
println("Locked file $lockId")
}
}
return true
} else
{
throw InvalidParameterException("ERROR: lockId or ownerId is null!")
}
}
This is the method for releasing:
override fun release(lockId: String?, ownerId: String?)
{
if (lockId != null)
{
lockedList.find { customLock -> customLock.Id == lockId }.apply {
if (this != null)
{
println("Unlocking file $lockId")
this.writeLock().unlock()
if (this.isWriteLocked)
{
throw Exception("ERROR: Unlocking failed!")
}
} else
{
throw Exception("ERROR: Lock not found!")
}
}
}
}
Please do not bother to talk about architecture, this one is dictated by the assignment. Also please ignore the ownerId and sequence variables.
EDIT: I tried using just a single lock and while not very efficient, it did work, so @gidds may be onto something, but neither ConcurrentHashMap nor ConcurrentLinkedQueue (it is simpler to replace a List with) solved the problem.
EDIT2: This is the new class where I used the ConcurrentHashMap. It still does not work correctly, can anyone please point out where I messed up? Thanks
class LockServer(port: Int) : LockConnector, RemoteException()
{
private val lockedList = ConcurrentHashMap<String, CustomLock>()
private var registry: Registry = LocateRegistry.createRegistry(port)
init
{
registry.bind(ServiceNames.LockService.toString(), UnicastRemoteObject.exportObject(this, port))
}
/**
* Method acquire() should block the multiple calls from the clients for each specific lockId string.
* It means when one client acquires the lock "A" and the "A" is not locked by any other clients,
* the method should record the lock and return true. If the "A" is already locked by any other client,
* the method is blocked and continues only after the lock "A" is released.
* (Note: Return value false is not used in this basic implementation.
* Parameters ownerId and sequence are also not used in this basic implementation.)
*/
override fun acquire(lockId: String?, ownerId: String?, sequence: Long): Boolean
{
if (lockId != null)
{
lockedList.computeIfPresent(lockId){id, value ->
println("Locking file $id Existing lock")
value.writeLock().lock()
println("Locked file $id")
return@computeIfPresent value
}
lockedList.computeIfAbsent(lockId){
val newLock = CustomLock(it)
println("Locking file $lockId")
newLock.writeLock().lock()
println("Locked file $lockId")
return@computeIfAbsent newLock
}
return true
} else
{
throw InvalidParameterException("ERROR: lockId or ownerId is null!")
}
}
/**
* Method release() should release the lock and unblock all waiting acquire() calls for the same lock.
* (Note: Parameter ownerId is not used in this basic implementation.)
*/
override fun release(lockId: String?, ownerId: String?)
{
if (lockId != null)
{
lockedList.computeIfPresent(lockId){ id, value ->
println("Unlocking file $id")
value.writeLock().unlock()
println("Unlocked file $id")
return@computeIfPresent value
}
}
}
/**
* Method stop() unbinds the current server object from the RMI registry and unexports it.
*/
override fun stop()
{
registry.unbind(ServiceNames.LockService.toString())
}
}
EDIT3: New implementation for acquire:
lockedList.compute(lockId){id, value ->
if (value == null)
{
println("Locking file $id")
val newLock = CustomLock(id)
newLock.writeLock().lock()
println("Locked file $id")
return@compute newLock
}
println("Locking file $id Existing lock")
value.writeLock().lock()
println("Locked file $id")
return@compute value
}
and for release:
println("Unlocking $lockId")
lockedList[lockId]!!.writeLock().unlock()
println("Unlocked $lockId")
Still the same failure
This may not be the problem of LockServer class, but the ones that are using it:
thread1:
acquire("file1")
acquire("file2")
release("file2")
release("file1")
thread2:
acquire("file2")
acquire("file1")
release("file1")
release("file2")
And it happened that the execution order was the following:
thread1.acquire("file1")
thread2.acquire("file2")
thread1.acquire("file2") //locked by thread2, waiting
thread2.acquire("file1") //locked by thread1... BOOM, deadlock!
UPD.:
Consider using tryLock() (possibly with some timeout) instead of simple lock() for existing locks:
fun tryAcquire(lockId: String?, timeout: Long, unit: TimeUnit): Boolean {
if (lockId != null) {
var success = false
lockedList.compute(lockId) { id, value ->
if (value == null) {
println("Locking file $id")
val newLock = CustomLock(id)
newLock.writeLock().lock()
success = true
println("Locked file $id")
return@compute newLock
}
println("Locking file $id Existing lock")
val lock = value.writeLock()
if (lock.tryLock() || lock.tryLock(timeout, unit)) {
success = true
println("Locked file $id")
}
return@compute value
}
return success
} else {
throw InvalidParameterException("ERROR: lockId or ownerId is null!")
}
}