ReentrantReadWriteLock.write dangerous

#1

Good day, I think that the ReentrantReadWriteLock.write() function is incorrectly written. Please correct me if I am wrong, but I believe that the rw lock is used in a way which allows multiple reader threads, but only one exclusive writer thread. The writer thread may only proceed when there are no readers.

However, Kotlin’s ReentrantReadWriteLock.write() forcefully unlocks all read locks (but leaves the readers running), then obtains the write lock (this is now possible as all read locks are released) and proceeds with the call. I believe that this violates the principle that the writer runs exclusively with no concurrent readers, and may produce dangerous threading issues.

#2

actually it just unlocks all read locks held by the current thread acquires the write lock and after executing the action re-acquries the read locks for the current thread again.
i didnt know you had to do that and find it suspicious that if a thread can acquire a read lock while holding the write lock why it needs to relinquish all held read locks when acquiring a write lock in the first place.

#3

why it needs to relinquish all held read locks when acquiring a write lock in the first place

This is simply the way how the ReentrantReadWriteLock works. Quoting from the javadoc: “a writer can acquire the read lock, but not vice-versa. Among other applications, reentrancy can be useful when write locks are held during calls or callbacks to methods that perform reads under read locks. If a reader tries to acquire the write lock it will never succeed.”

Now, back to my point. There are indeed multiple use cases for rw lock. In the ReentrantReadWriteLock.java class javadoc there is indeed the CachedData example, for which the Kotlin’s write fun may be usable (although please notice that the CachedData example only releases its own thread’s lock, and not all locks as it is done in write).
However, imagine the following case:

class ThreadSafeList {
    private val lock = ReentrantReadWriteLock()
    private val list = ArrayList<String>()
    fun get(index: Int) = lock.read { list[index] }
    fun add(str: String) = lock.write { list.add(str) }
}

This is a perfectly valid use case of a rw lock. However, in Kotlin’s impl the write thread will release all read locks and enter the write block, even when there are multiple read threads still ongoing in the read block. An unfortunate race condition may happen:

  1. list.add() needs to increase its internal array capacity, allocates a new array full of nulls and gradually starts to copy the old one over the newly allocated one
  2. because of how the processor cache works, read thread may see those operations in random (even reverted order). So it is possible to read null from the newly created array.

So, the get() fun will randomly read nulls under a heavy load, which makes this bug extremely nasty - it is hard to simulate and only occurs randomly. Be very careful when working with threads :slight_smile:

#4

If we rewrite the class as follows:

class ThreadSafeList {
    private val lock = ReentrantReadWriteLock()
    private val list = ArrayList<String>()
    fun get(index: Int) = lock.read { list[index] }
    fun add(str: String) {
        lock.writeLock().lock()
        try { list.add(str) } finally { lock.writeLock().unlock() }
    }
}

The problem goes away, simply because the write lock obtain operation will wait until all readers are finished (and also because the unlock-lock ops forms a happens-before relationship, but that’s not relevant now). The main problem here is that I would expect the write Kotlin fun do to exactly this: not to mess with the read locks, but simply obtain a write lock, waiting until it becomes available.

#5

write releases the read locks of the current thread and acquires them later again. so no it does not release the read locks of other threads.
readHoldCount is a thread local value. so if the thread of the write operation holds any read locks they will be temporartily relinquished and then before the write lock is released re-acquired.
your quote from javadocs “If a reader tries to acquire the write lock it will never succeed” actually clarifies the code. i would have assumed that if the current thread is the only reader it can acquire/promote to the write lock.

#6

I knew there is something I do not understand :slight_smile: Thank you for clarification, you are of course correct. It’s just that I expected to see only write lock code in the write method, and that read lock magic confused me.

Thanks again, I withdraw my case :slight_smile: