rework container locking

This commit is contained in:
Bixilon 2023-01-13 14:17:42 +01:00
parent 784453a754
commit 5181e8ed00
No known key found for this signature in database
GPG Key ID: 5CAD791931B09AC4
11 changed files with 185 additions and 101 deletions

View File

@ -45,7 +45,7 @@ class ContainerLockingTest {
container.lock()
container.commit()
assertEquals(container.revision, 0L)
assertEquals(container.revision, 1L)
}
fun verifyRevisionSingle() {
@ -57,7 +57,7 @@ class ContainerLockingTest {
assertEquals(container.revision, 1L)
assertNotNull(container[0])
container[0]!!.item.decreaseCount()
container[0]!!.item.increaseCount()
assertEquals(container.revision, 2L)
assertEquals(container[0], ItemStack(AppleTestO.item, count = 16))
}

View File

@ -23,7 +23,7 @@ import de.bixilon.minosoft.test.ITUtil
import org.testng.Assert
import org.testng.annotations.Test
@Test(groups = ["pixlyzer"], dependsOnGroups = ["version"], singleThreaded = false, threadPoolSize = 8, priority = Int.MAX_VALUE)
@Test(groups = ["pixlyzer"], dependsOnGroups = ["version"], singleThreaded = false, threadPoolSize = 8, priority = Int.MAX_VALUE, timeOut = 10000L)
class PixLyzerLoadingTest {
private fun VersionRegistry.test() {

View File

@ -14,9 +14,8 @@
package de.bixilon.minosoft.data.container
import de.bixilon.kutil.collections.CollectionUtil.synchronizedBiMapOf
import de.bixilon.kutil.collections.CollectionUtil.toSynchronizedMap
import de.bixilon.kutil.collections.map.bi.SynchronizedBiMap
import de.bixilon.kutil.concurrent.lock.simple.SimpleLock
import de.bixilon.kutil.concurrent.lock.thread.ThreadLock
import de.bixilon.kutil.observer.DataObserver.Companion.observe
import de.bixilon.kutil.observer.DataObserver.Companion.observed
import de.bixilon.kutil.observer.map.MapObserver.Companion.observedMap
@ -45,7 +44,7 @@ open class Container(
) : Iterable<Map.Entry<Int, ItemStack>> {
@Deprecated("Should not be accessed directly")
val slots: MutableMap<Int, ItemStack> by observedMap(Int2ObjectOpenHashMap())
val lock = SimpleLock()
val lock = ThreadLock()
var propertiesRevision by observed(0L)
var revision by observed(0L)
var serverRevision = 0
@ -62,29 +61,26 @@ open class Container(
this::floatingItem.observe(this) { it?.holder?.container = this }
}
fun _validate() {
var itemsRemoved = 0
if (floatingItem?._valid == false) {
floatingItem = null
itemsRemoved++
}
for ((slot, itemStack) in slots.toSynchronizedMap()) {
if (itemStack._valid) {
continue
}
_remove(slot)
itemStack.holder?.container = null
itemsRemoved++
}
if (itemsRemoved > 0) {
revision++
}
}
var edit: ContainerEdit? = null
fun validate() {
lock.lock()
_validate()
lock.unlock()
if (floatingItem?._valid == false) {
floatingItem = null
edit?.addChange()
}
val iterator = slots.iterator()
for ((slot, stack) in iterator) {
if (stack._valid) {
continue
}
stack.holder?.container = null
iterator.remove()
edit?.addChange()
}
internalCommit()
}
open fun getSlotType(slotId: Int): SlotType? = DefaultSlotType
@ -108,6 +104,7 @@ open class Container(
}
}
@Deprecated("")
open fun _remove(slotId: Int): ItemStack? {
val stack = slots.remove(slotId) ?: return null
stack.holder?.container = null
@ -117,24 +114,19 @@ open class Container(
open fun remove(slotId: Int): ItemStack? {
lock.lock()
val remove = _remove(slotId)
lock.unlock()
if (remove != null) {
revision++
edit?.addChange()
}
internalCommit()
return remove
}
open operator fun set(slotId: Int, itemStack: ItemStack?) {
try {
lock.lock()
if (!_set(slotId, itemStack)) {
return
}
} finally {
lock.unlock()
lock.lock()
if (_set(slotId, itemStack)) {
edit?.addChange()
}
revision++
internalCommit()
}
open fun _set(slotId: Int, itemStack: ItemStack?): Boolean {
@ -166,30 +158,27 @@ open class Container(
return
}
lock.lock()
var changes = 0
for ((slotId, itemStack) in slots) {
if (_set(slotId, itemStack)) {
changes++
edit?.addChange()
}
}
lock.unlock()
if (changes > 0) {
revision++
}
internalCommit()
}
fun _clear() {
for (stack in slots.values) {
stack.holder?.container = null
}
slots.clear()
if (slots.isNotEmpty()) {
edit?.addChange()
}
}
fun clear() {
lock.lock()
_clear()
lock.unlock()
revision++
internalCommit()
}
@Synchronized
@ -241,8 +230,28 @@ open class Container(
return slots.iterator()
}
fun commit() {
fun lock() {
lock.lock()
if (edit == null) {
edit = ContainerEdit()
}
}
fun internalCommit() {
val edit = this.edit
lock.unlock()
if (edit == null) {
revision++
}
}
fun commit() {
val edit = this.edit ?: throw IllegalStateException("Not in bulk edit mode!")
validate()
lock.unlock()
for (slot in edit.slots) {
slot.revision++
}
revision++
}

View File

@ -0,0 +1,25 @@
/*
* Minosoft
* Copyright (C) 2020-2023 Moritz Zwerger
*
* This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License along with this program. If not, see <https://www.gnu.org/licenses/>.
*
* This software is not affiliated with Mojang AB, the original developer of Minecraft.
*/
package de.bixilon.minosoft.data.container
import de.bixilon.minosoft.data.container.stack.ItemStack
class ContainerEdit {
val slots: MutableSet<ItemStack> = mutableSetOf()
var changes = 0
fun addChange() {
changes++
}
}

View File

@ -1,6 +1,6 @@
/*
* Minosoft
* Copyright (C) 2022 Moritz Zwerger
* Copyright (C) 2020-2023 Moritz Zwerger
*
* This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version.
*
@ -33,8 +33,8 @@ class InventoryDelegate<T>(
}
override fun setValue(thisRef: Any, property: KProperty<*>, value: T) {
stack.lock()
stack.lock.lock()
field.setValue(thisRef, property, value)
stack.commit()
stack.internalCommit()
}
}

View File

@ -0,0 +1,22 @@
/*
* Minosoft
* Copyright (C) 2020-2023 Moritz Zwerger
*
* This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License along with this program. If not, see <https://www.gnu.org/licenses/>.
*
* This software is not affiliated with Mojang AB, the original developer of Minecraft.
*/
package de.bixilon.minosoft.data.container
class SlotEdit {
var changes = 0
fun addChange() {
changes++
}
}

View File

@ -1,6 +1,6 @@
/*
* Minosoft
* Copyright (C) 2020-2022 Moritz Zwerger
* Copyright (C) 2020-2023 Moritz Zwerger
*
* This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version.
*
@ -29,7 +29,7 @@ class FastMoveContainerAction(
override fun invoke(connection: PlayConnection, containerId: Int, container: Container) {
// ToDo: minecraft always sends a packet
val source = container.slots[slot] ?: return
container.lock.lock()
container.lock()
try {
val sourceSection = container.getSection(slot) ?: Int.MAX_VALUE
@ -38,7 +38,6 @@ class FastMoveContainerAction(
for ((index, section) in container.sections.withIndex()) {
if (index == sourceSection) {
// we don't want to swap into the same section, that is just useless
// ToDo: Is this vanilla behavior?
continue
}
if (section.count == 0) {
@ -47,7 +46,7 @@ class FastMoveContainerAction(
val list = IntArrayList()
targets += Pair(section, list)
for (slot in section.iterator()) {
val content = container.slots[slot]
val content = container[slot]
if (content != null && !source.matches(content)) { // only check slots that are not empty
continue
}
@ -64,13 +63,12 @@ class FastMoveContainerAction(
sections@ for ((type, list) in targets) {
val putting = if (type.fillReversed) list.reversed().iterator() else list.intIterator()
for (slot in putting) {
val content = container.slots[slot] ?: continue // filling will be done one step afterwards
val countToPut = if (source.item._count + content.item._count > maxStack) maxStack - content.item._count else source.item._count
source.item._count -= countToPut
content.item._count += countToPut
content.commit(false)
val content = container[slot] ?: continue // filling will be done one step afterwards
val countToPut = if (source.item.count + content.item.count > maxStack) maxStack - content.item.count else source.item.count
source.item.count -= countToPut
content.item.count += countToPut
changes[slot] = content
if (source.item._count <= 0) {
if (source.item.count <= 0) {
changes[this.slot] = null
break@sections
}
@ -79,19 +77,19 @@ class FastMoveContainerAction(
}
sections@ for ((type, list) in targets) {
if (source.item._count <= 0) {
if (source.item.count <= 0) {
break
}
val putting = if (type.fillReversed) list.reversed().iterator() else list.intIterator()
for (slot in putting) {
val content = container.slots[slot]
val content = container[slot]
if (content != null) {
continue
}
changes[slot] = source
changes[this.slot] = null
container._set(slot, source)
container._set(this.slot, null)
container[slot] = source
container[this.slot] = null
break@sections
}
}
@ -99,7 +97,6 @@ class FastMoveContainerAction(
connection.sendPacket(ContainerClickC2SP(containerId, container.serverRevision, this.slot, 1, 0, container.createAction(this), changes, null))
} finally {
container.commit()
container._validate()
}
}
}

View File

@ -1,6 +1,6 @@
/*
* Minosoft
* Copyright (C) 2020-2022 Moritz Zwerger
* Copyright (C) 2020-2023 Moritz Zwerger
*
* This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version.
*
@ -30,14 +30,14 @@ class PickAllContainerAction(
override fun invoke(connection: PlayConnection, containerId: Int, container: Container) {
// TODO (1.18.2) minecraft always sends a packet
container.lock.lock()
container.lock()
try {
val previous = container.slots[slot]
val previous = container[slot]
val floating = container.floatingItem?.copy()
if (previous != null || floating == null) {
return
}
var countLeft = floating.item.item.maxStackSize - floating.item._count
var countLeft = floating.item.item.maxStackSize - floating.item.count
val changes: Int2ObjectMap<ItemStack?> = Int2ObjectOpenHashMap()
for ((slotId, slot) in container.slots) {
if (!floating.matches(slot)) {
@ -46,10 +46,10 @@ class PickAllContainerAction(
if (container.getSlotType(slotId)?.canRemove(container, slotId, slot) != true) {
continue
}
val countToRemove = minOf(slot.item._count, countLeft)
slot.item._count -= countToRemove
val countToRemove = minOf(slot.item.count, countLeft)
slot.item.count -= countToRemove
countLeft -= countToRemove
floating.item._count += countToRemove
floating.item.count += countToRemove
if (slot._valid) {
changes[slotId] = slot
} else {
@ -59,7 +59,7 @@ class PickAllContainerAction(
break
}
}
container._validate()
container.validate()
if (floating == container.floatingItem) {
// no change
return

View File

@ -1,6 +1,6 @@
/*
* Minosoft
* Copyright (C) 2020-2022 Moritz Zwerger
* Copyright (C) 2020-2023 Moritz Zwerger
*
* This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version.
*
@ -36,19 +36,19 @@ class SimpleContainerAction(
if (count == ContainerCounts.ALL) {
floatingItem = item.copy()
item.item.count = 0
container._remove(slot)
container[slot] = null
} else {
// half
val stayCount = item.item.count / 2
item.item.count = stayCount
floatingItem = previous.copy(count = previous.item.count - stayCount)
}
container.floatingItem = floatingItem
connection.sendPacket(ContainerClickC2SP(containerId, container.serverRevision, slot, 0, count.ordinal, container.createAction(this), slotsOf(slot to item), floatingItem))
}
private fun putItem(connection: PlayConnection, containerId: Int, container: Container, floatingItem: ItemStack) {
var target = container.slots[slot]
if (slot == null) {
// slot id is null, we are not targeting anything
// -> drop item into the void
@ -59,6 +59,7 @@ class SimpleContainerAction(
}
return connection.sendPacket(ContainerClickC2SP(containerId, container.serverRevision, null, 0, count.ordinal, container.createAction(this), slotsOf(), null))
}
var target = container[slot]
val slotType = container.getSlotType(slot)
val matches = floatingItem.matches(target)
@ -66,23 +67,22 @@ class SimpleContainerAction(
// we can remove or merge the item
if (slotType?.canPut(container, slot, floatingItem) == true) {
// merge
val subtract = if (count == ContainerCounts.ALL) minOf(target.item.item.maxStackSize - target.item._count, floatingItem.item._count) else 1
if (subtract == 0 || target.item._count + subtract > target.item.item.maxStackSize) {
val subtract = if (count == ContainerCounts.ALL) minOf(target.item.item.maxStackSize - target.item.count, floatingItem.item.count) else 1
if (subtract == 0 || target.item.count + subtract > target.item.item.maxStackSize) {
return
}
target.item._count += subtract
floatingItem.item._count -= subtract
target.item.count += subtract
floatingItem.item.count -= subtract
} else if (slotType?.canRemove(container, slot, floatingItem) == true) {
// remove only (e.g. crafting result)
// ToDo: respect count (part or all)
val subtract = minOf(floatingItem.item.item.maxStackSize - floatingItem.item._count, target.item._count)
val subtract = minOf(floatingItem.item.item.maxStackSize - floatingItem.item.count, target.item.count)
if (subtract == 0) {
return
}
target.item._count -= subtract
floatingItem.item._count += subtract
target.item.count -= subtract
floatingItem.item.count += subtract
}
target.commit(false)
if (floatingItem._valid) {
container.floatingItem = floatingItem
@ -104,20 +104,20 @@ class SimpleContainerAction(
// swap
if (count == ContainerCounts.ALL || (!matches && target != null)) {
container.floatingItem = target
container._set(slot, floatingItem)
container[slot] = floatingItem
connection.sendPacket(ContainerClickC2SP(containerId, container.serverRevision, slot, 0, count.ordinal, container.createAction(this), slotsOf(slot to floatingItem), target))
} else {
floatingItem.item._count--
floatingItem.item.count--
container.floatingItem = floatingItem
target = floatingItem.copy(count = 1)
container._set(slot, target)
container[slot] = target
connection.sendPacket(ContainerClickC2SP(containerId, container.serverRevision, slot, 0, count.ordinal, container.createAction(this), slotsOf(slot to target), floatingItem))
}
}
override fun invoke(connection: PlayConnection, containerId: Int, container: Container) {
try {
container.lock.lock()
container.lock()
val floatingItem = container.floatingItem?.copy() ?: return pickItem(connection, containerId, container)
return putItem(connection, containerId, container, floatingItem)
} finally {

View File

@ -1,6 +1,6 @@
/*
* Minosoft
* Copyright (C) 2020-2022 Moritz Zwerger
* Copyright (C) 2020-2023 Moritz Zwerger
*
* This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version.
*
@ -13,10 +13,12 @@
package de.bixilon.minosoft.data.container.stack
import de.bixilon.kutil.concurrent.lock.simple.ParentLock
import de.bixilon.kutil.concurrent.lock.thread.ThreadLock
import de.bixilon.kutil.json.JsonObject
import de.bixilon.kutil.json.MutableJsonObject
import de.bixilon.kutil.observer.DataObserver.Companion.observed
import de.bixilon.minosoft.data.Rarities
import de.bixilon.minosoft.data.container.SlotEdit
import de.bixilon.minosoft.data.container.stack.property.*
import de.bixilon.minosoft.data.registries.item.items.Item
import de.bixilon.minosoft.data.text.ChatComponent
@ -24,10 +26,12 @@ import de.bixilon.minosoft.protocol.network.connection.play.PlayConnection
import java.util.*
class ItemStack {
val lock = ParentLock()
val lock = ParentLock(lock = ThreadLock())
val item: ItemProperty
var holder: HolderProperty? = null
var edit: SlotEdit? = null
var _display: DisplayProperty? = null
private set
val display: DisplayProperty
@ -122,7 +126,7 @@ class ItemStack {
val displayName: ChatComponent
get() {
_display?.customDisplayName?.let { return it }
item.item.translationKey?.let {
item.item.translationKey.let {
val language = holder?.connection?.language ?: return@let
val translated = language.translate(it)
rarity.color.let { color -> translated.setFallbackColor(color) }
@ -196,17 +200,44 @@ class ItemStack {
fun lock() {
lock.lock()
if (holder?.container?.edit != null) {
return
}
if (edit == null) {
edit = SlotEdit()
}
}
fun commit(unlock: Boolean = true) {
fun internalCommit() {
val container = holder?.container
if (!_valid) {
holder?.container?._validate()
container?.validate()
}
if (unlock) {
lock.unlock()
val containerEdit = container?.edit
containerEdit?.let { it.addChange(); it.slots += this }
val edit = edit
if (edit != null) {
return
}
lock.unlock()
if (containerEdit != null) {
return
}
revision++
holder?.container?.apply { revision++ } // increase revision after unlock to prevent deadlock
container?.apply { revision++ }
}
fun commit() {
if (!_valid) {
holder?.container?.validate()
}
val edit = edit ?: throw IllegalStateException("Not in edit mode!")
lock.unlock()
if (edit.changes > 0) {
revision++
holder?.container?.apply { revision++ }
}
}
override fun hashCode(): Int {

View File

@ -1,6 +1,6 @@
/*
* Minosoft
* Copyright (C) 2020-2022 Moritz Zwerger
* Copyright (C) 2020-2023 Moritz Zwerger
*
* This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version.
*
@ -28,15 +28,15 @@ class ItemProperty(
fun decreaseCount() {
stack.lock()
stack.lock.lock()
_count -= 1
stack.commit()
stack.internalCommit()
}
fun increaseCount() {
stack.lock()
stack.lock.lock()
_count += 1
stack.commit()
stack.internalCommit()
}
override fun isDefault(): Boolean = false