Today is day 4 of trying to fix a dupe glitch in my game so I am reaching out for help.
Essentially, I now believe that the dupe is originating from unsafe handling of variables in a priority queue. Specifically I think that the nested while loop below sometimes starts another iteration over itself before queue:pop() has safely removed the top element from the queue. BTW the dupe is happening when processQueue() is being called and a lot of items are being added to the queue.
I am not super familiar with coroutines so please tell me if the system below is jank as heck and I should be using wrap() or something like that. Any help is appreciated!
local serverFunctions = require(game.ServerScriptService.modules.serverFunctions)
local inventoryRequests = require(game.ServerScriptService.modules.inventoryRequests)
local queue = inventoryRequests.PriorityQueue
local inventoryEvent = game.ReplicatedStorage.events.inventoryEvent
local processor = {}
local function job()
warn("THREAD CREATED")
while true do
while #queue.heap > 0 do
local request = queue:pop()
local success, changeTable = serverFunctions.plrToStorage(request)
inventoryEvent:FireClient(request[1], success, changeTable)
end
coroutine.yield()
end
end
local processTask = nil
function processor.processQueue()
if not processTask or coroutine.status(processTask) == "dead" then
warn("REVIVING")
processTask = coroutine.create(job)
end
if coroutine.status(processTask) == "running" then
warn("Processor queue is already running.")
return
end
coroutine.resume(processTask)
end
return processor
I apologize I should have specified what I seeing in console. The console only ever shows THREAD CREATED 1 time. I should also add that the dupe is rare and only happens probably every 25 iterations of the while loop? The whole thing is kinda strange and I am at a loss for what to do. THis is the only place that could be the issue.
I kind of just hoped somebody had seen something like this before cuz i t seems to be like a Studio issue rather than a code one (although my code prob isn’t the best either).
Hi! in your processQueue function, why are you checking if it’s running and returning if it is. Wouldn’t it be more efficient to check if the coroutine.status is suspended then resuming it from there?
To address your problem I haven’t had many duping issues myself, but in your InventoryEvent, on the client side, are you checking if success is true? if it is, then you can create a dictionary on the client and assign them random ids. That way, you could check if the ID has already been process on the client once the new data gets to them.
Can we see the implementation for the priority queue? Also, are you 100% sure the dupe is happening from the processing and not other code mistakenly adding a dupe to the queue? Have you tried logging the additions to the queue and logging what gets processed in order to find the discrepancy?
I don’t really see anything wrong with the code you’ve posted. The one thing I like to do in cases like this is move all the things I’m going to process into a different table. That way, if my processing code yields for any reason, there’s no risk that the table I’m iterating over will be modified mid-loop I doubt it would fix anything here, but it’s just a way for me to keep the processing separate from the addition of requests.
Not your fault as it is poorly explained, but calling coroutine.status on a “running” thread outside of that said thread will return suspended (unfortunately).
There is no need to use coroutines in this case. Because of the issue above, calling coroutine.resume on an already running thread will have unintended consequences. You can achieve most of, if not everything, without the use of coroutine.create/resume and coroutine.wrap. In your case, just create a new thread with task.spawn whenever you call processQueue, and drop the request when it’s already running.
local jobThread: thread? = nil
function processor.processQueue(): ()
if jobThread == nil and #queue.heap > 0 then
print("Started job thread")
jobThread = task.spawn(function(): ()
while #queue.heap > 0 do
local request = queue:pop()
local success, changeTable = serverFunctions.plrToStorage(request)
inventoryEvent:FireClient(request[1], success, changeTable)
end
jobThread = nil
end)
else
warn("Job thread is running")
end
end
Can you elaborate on this? From what I understand, if you’re calling coroutine.status on a separate thread, then it’s impossible for that thread to be running and it must be either suspended, dead, or waiting on the yield of another thread. I don’t think there’s any weird behavior regarding that function.
So I found the problem. Turns out the issue was originating from the storage-to-plr logic instead of the plr-to-storage logic as I thought before. The reason I didn’t notice this is because I was only looking at the UI data on the client (which was totally idiotic looking back). So no wonder my attempts to change plr-to-storage didn’t do anything!
Needless to say, I explored all of your suggestions and they were what kept me motivated to eventually solve the bug. So even though none of them actually helped (because I didn’t give you relevant information to what happened to be the real problem), you all helped!