Prevent deadlock from other mods in item merge

[MartijnMuijsers]

Some mods change item behavior and may want to execute things when item are merged, the count is changed or when an entity is discarded, and may try to acquire another lock, but if a thread holding that lock causes an item merge (by moving items from world to world for example) the code will deadlock.

Simple solution to make this change safe for mods: if the lock is not free when attempting merging, just skip the merge. This barely ever happens in vanilla (and when it does, it doesn't matter because those rare cases will just be merged anyway the next tick), but at least the code can now never deadlock and give headaches in the future.

[himekifee]

Deadlock I described before was caused by my minor mistake. Have you encountered a deadlock with the latest commit? I think your pr will help with mspt by spliting merging into a lot of later ticks but I don't think it will be that much of improvement as the synced code is simple.

[himekifee]

If mods changes merging behaviour, I think they won't change the current processing entity list and will be scheduled on the next tick. But if not then it's problematic. What do you think

[MartijnMuijsers]

A mod could change the behavior of ItemEntity.merge (or .discard inside of that) which is called inside of tryToMerge, while locked. So it could easily do something that calls another lock B. Another thread having that other lock B and then trying to call tryToMerge seems unlikely because tryToMerge is not called often.

I did not encounter a deadlock but just saw this as a possibility since the call of merge inside of the lock felt a bit dangerous to me. So I just figured this tiny change could make it safe.

I didn't even think about performance! But now you said it, I think it won't matter in almost all cases, except when someone drops a whole ton of item entities in the same spot, because they will all try to merge regardless of different types. So for a double chest that is broken I could see it blocking almost the whole thread pool if almost every thread is working on one of those entities and waiting for the lock. In that case this may make the items merge at a later unpredictable time, but the merging will be stable and I guess as lagless as possible.

And to the last point, tryToMerge appears to also be called from changeDimension, so that's why I figured a mod could call it. If a mod changes the processing entity list we are epicly screwed anyway, I wasn't thinking about that haha!

[MartijnMuijsers]

To prevent items not merging because the lock is too busy, the best solution is using a lock per entity. It's 2am here but I'll post something tomorrow so you can see!

[himekifee]

Ya, also 1 am here. Gn, sleep tight.

[MartijnMuijsers]

I already closed my laptop but I just solved it in my head so that:

• Almost all items merge within 1 tick regardless of how many there are
• No thread is ever blocked
• Almost no performance cost
• Item merges are fully safe

Will post tomorrow! Good night~