Race condition in develop branch item reader vs. item request handler?

[palmettos]

I'm developing a program that reads on an interval basis from AllItemsRequestHandler. The program stores its own inventory state and diffs it with incoming data from AllItemsRequestHandler before pushing the delta to a web server. What I've noticed is that occasionally items that are actually equipped aren't returned in the payload from the request handler. My assumption is that the item reader thread is still writing to the character state while the request handler is reading it. This seemingly makes accurate delta compression impossible. I don't have a ton of time to study the develop source, nor am I particularly experienced in C#, so I figured I'd post this as you know a lot more about the source than I do :).

[qhris]

Hey!
While I haven't tested it, it seems that this could be one of the reasons, though I'm sure it could be some data race due to the reader being used from two threads (the data reader thread and the request handling thread).
If you have time to try it, try adding a lock in the ItemSlotAction and RunReadOperation methods so only one of them can do work at the same time.

[palmettos]

Hey, thanks for the really quick response and for pointing me in the right direction. I'll try what you suggested and follow up on anything I discover. If I end up fixing it I'll open a pull request. Thanks!

[palmettos]

I tried locking the sections of code you suggested, but the problem persists. I added some better debugging output from my program to try to help make sense of the issue.

When swapping weapons from a fully equipped character, these are the resulting json objects and diff with my program's inventory state:

Immediately after pressing W:
json: [{ u'ItemName': u'Buckler', u'Location': 12, u'Properties': []}]

diff: ['removed Cap', 'removed Amulet of Energy', 'removed Brimstone Mantle Quilted Armor', 'removed Ring of Craftmanship', 'removed Bronze Ring', 'removed Sash of Thorns', 'removed Ethereal Boots of Dexterity', 'removed Leather Gloves of Dexterity', 'removed Scimitar']

And the next json object read out from the pipe:
json: [{ u'ItemName': u'Buckler', u'Location': 5, u'Properties': []}, { u'ItemName': u'Scimitar', u'Location': 4, u'Properties': [u'Adds 3-4 fire damage', u'Socketed (2)']}, { u'ItemName': u'Brimstone Mantle Quilted Armor', u'Location': 3, u'Properties': [ u'+2 to Life', u'Lightning Resist +7%', u'Fire Resist +7%', u'Poison Resist +10%', u'Damage Reduced by 1', u'Poison Length Reduced by 25%']}, { u'ItemName': u'Sash of Thorns', u'Location': 8, u'Properties': [u'Attacker Takes Damage of 1']}, { u'ItemName': u'Ring of Craftmanship', u'Location': 6, u'Properties': [u'+1 to Maximum Damage']}, { u'ItemName': u'Bronze Ring', u'Location': 7, u'Properties': [u'+17 to Attack Rating']}, { u'ItemName': u'Ethereal Boots of Dexterity', u'Location': 9, u'Properties': [u'+1 to Dexterity']}, { u'ItemName': u'Leather Gloves of Dexterity', u'Location': 10, u'Properties': [u'+1 to Dexterity']}, { u'ItemName': u'Cap', u'Location': 1, u'Properties': []}, { u'ItemName': u'Amulet of Energy', u'Location': 2, u'Properties': [u'+1 to Energy']}]

diff: ['removed Buckler', 'added Buckler', 'added Scimitar', 'added Brimstone Mantle Quilted Armor', 'added Sash of Thorns', 'added Ring of Craftmanship', 'added Bronze Ring', 'added Ethereal Boots of Dexterity', 'added Leather Gloves of Dexterity', 'added Cap', 'added Amulet of Energy']

My only guess now is that it's a problem with reading the game's memory, like Diablo 2 might be writing to those addresses while the data reader is reading it. For the time being, I think I've come up with a trashy fix to make my delta compression more reliable despite the issue, but I'd be happy to investigate further if you can offer any more insight.

[Lectem]

I don't know the code being used, but I'm pretty sure it comes from the fact the game is writing to the memory while you're reading it. Easiest might be to trigger the read based on a certain function being called (beginning of draw ?).
The other solution would be to validate the data over a few frames

[qhris]

I agree with @Lectem, this is a big drawback with how the data reads currently work. I'm not entirely sure how adding such a trigger is done and what kind of implications it has. In the end it's just limited what we can do with just data reads (via ReadProcessMemory).

It's unfortunate that the read locks didn't help much, but at this point I'd say there's not much that could be changed. I'll let this issue be open for a few more days, in case you can come up with something more. I'd say try out what @Lectem suggested with the data validation, seems like the correct way to go here.

[palmettos]

Yeah, the solution I've come up with should be enough for my needs. I'm just going to use a temporary inventory state and a final inventory state. Every time the temporary state changes, a timer resets and starts counting up. The final state will only diff with the temporary state if it's remained unchanged for a couple seconds. As far as triggering the read as @Lectem suggests, I think this could be achieved with a library like Deviare, where you'd hook some graphical function that Diablo 2 calls in a DLL like direct3d or glide, and trigger all memory reads at that point. That way you'd be sure all Diablo 2 memory updates had already occurred for that frame.