Wrong choice of Lock

[SimonDanisch]

We were running into deadlocks with LRUCache, and I think it boils down to SpinLock not being the best choice here. From the docs:

help?> Threads.SpinLock
  SpinLock()

  Create a non-reentrant, test-and-test-and-set spin lock. Recursive use will result in a deadlock. This kind of lock
  should only be used around code that takes little time to execute and does not block (e.g. perform I/O). In general,
  ReentrantLock should be used instead.

  Each lock must be matched with an unlock.

  Test-and-test-and-set spin locks are quickest up to about 30ish contending threads. If you have more contention than
  that, different synchronization approaches should be considered.

Would it be possible to use ReentrantLock instead? It does seem to fix our issue at least.

[Jutho]

Do you understand how this current spinlock is being called recursively, which should be the origin of the deadlocks. Do you reaccess the cache from within the user function supplied to get!?

I only now notice that I never registered the latest commit, a.k.a. v1.2 , which moves all user code out of the part within which the lock is retained. Can you try whether just using master version of LRUCache fixes your problem? If so, then I would register as is, with SpinLock, which should be the fastest option. If not, then switching to ReentrantLock is probably the best solution.

[SimonDanisch]

I think, a yield inside the user function makes it jump out of the function and may yield to a function, that again tries to load from the Cache - that was my explanation at least, since it should be strictly non recursive code.

[SimonDanisch]

But I don't think I tried the newest version, so maybe that actually fixes it (yield should make it jump to another task, so actually locking correctly with SpinLock?).

[Jutho]

With the latest master version the lock is freed before the user function default() is ran, so the actual code within the lock ... unlock block is very short, at most checking whether the key exists and if so, getting the value. It would be great if you could try. I implemented this commit because we were also seeing deadlocks in certain cases, but then I forgot to actually register the new version.

[Jutho]

Any update here? Did you find a chance to rerun your code with the master version and see if this resolves the deadlocks?

[SimonDanisch]

I'll check next week ;)

[SimonDanisch]

I can confirm, that master solves this issue! Let's close the issue once master is tagged!

[Jutho]

1.2 should be tagged now