L2J Server

I was just glancing through the crafting code, and I have some doubts about ingredients availability check.

It goes like this:

1. The check is done with RecipeController$RecipeItemMaker.listItems(boolean remove) method, with the "remove" parameter indicating if ingredients shall be taken out of the inventory or not.
2. First time, it's run in RecipeItemMaker constructor ("remove" parameter = false), as one of verification steps.
3. Second time, it's run just before the success/fail check, in RecipeItemMaker.finishCrafting() method, this time with "remove" parameter = true.

Now, listItem() method looks like this: Now, for me it looks like a classical critical section, that is not thread-safe. It is theoretically possible for a player (i.e. another server thread) to remove the ingredients just after the "foreach" part, but before the "remove" part. Of course that it *is* hard, but it is *not* impossible.

I believe introducing synchronization to the critical section mentioned is required. Please correct me if I'm wrong, I'm definitely not an L2j guru.

in theory yes but we dont make bank soft so in some places are synces ignored for performance benefit. This code is quite old maybe there would be a better way to handle it now (it dont check even destroyitem return value..). Also this will not be able with single packet executor.

By no means I consider this thing a critical issue or something. It's more just a tip to point your attention towards "proper" multi-threaded coding. I mean, from what I understand, L2j was created as an educational project, so this is like educational stuff. We all want to be *good* software devs (-;