[Lldb-commits] [PATCH] D126240: [lldb] Tighten the scope of a couple of locks in DataFormatters.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D126240#3564913 <https://reviews.llvm.org/D126240#3564913>, @jgorbe wrote:

> I've been experimenting a little bit and it seems that I can avoid the deadlocks I was seeing by applying only the modification to `TypeCategoryMap::Add`. This would avoid the problem that @hawkinsw pointed out in my change to `FormatManager::GetCategoryForLanguage`. However, the following problem could still happen with the change to `TypeCategoryMap::Add`:
>
> 1. Thread A acquires the lock, runs `m_map[key1] = value1`, and releases the lock
> 2. Thread B acquires the lock, runs `m_map[key2] = value2`, and releases the lock
> 3. Thread A runs `listener->Changed()`
> 4. Thread B runs `listener->Changed()`
>
> So we would have two changes, the listener would be called twice, but both listener calls would see the same state with the two changes already applied, instead of observing each change individually. I'll keep thinking to see if I can fix this elsewhere.

[disclaimer: I suggested this approach offline]

Yes, that could happen, but is that a problem? That sequence of actions is indistinguishable (to the object receiving the Changed notifications) from an action which modifies two keys, followed by a no-op action.

I know we currently don't have such actions, but I don't see why should the receiver care about that. The notifications don't carry any information about the change anyway, so the only way it could notice that is if it made a snapshot of the state each time it is called and then done a diff. If we really wanted it to receive granular update informations like this, we would have designed the interface differently.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126240/new/

https://reviews.llvm.org/D126240