[QSRlib] request_qsrs not thread safe
Closed this issue · 10 comments
When requesting qsrs from multiple programmes at the same time, the reset command is not thread safe and sets the request message to None. What is that good for in the first place? The values are always overridden when the function is called anyway. @yianni is this used for anything or can I just remove that all together? If so, I can fix that together with #83
TL'DR: Yes remove.
The original idea was that if no data was passed then the previous data would be used... pseudocode example
qsrlib.request_qsrs(data, "qtcb")
qsrlib.request_qsrs(None, "rcc3") # uses previous data
I opted out to never use that as it is a very dangerous thing to get non-sense results and difficult to debug from the side of the user, hence I always "force" the passing of data at every request now (or at least I hope I have done so by always overwriting them), i.e. ignore no reset should ever occur.
OK, will fix.
Basically it was aimed to have the for loop on the client side rather than on the server. I prefer having it on the server side though for reasons above.
Makes sense. But I agree that the resulting risks outweigh the benefits. I'll remove it.
OK, I had a look on the reset in a bit more detail... Memory efficiency? As data should be released and memory should be freed, without it the previous data passed remain in the server memory.
But I don't need the member variable at all then, do I? I can just remove self.timestamp_request_received and self.request_message and the garbage collection should take care of that. Shouldn't it?
So basically reset is default True for memory efficiency... A typical cycle should look like:
request_qsrs(data)
compute_qsrs(data)
clean_data_on_server # purpose of reset
etc.
reset=False would keep previous data allowing computation of new qsr on same data (bad and dangerous thing for reasons we agreed above)
But I don't need the member variable at all then, do I? I can just remove self.timestamp_request_received and self.request_message and the garbage collection should take care of that. Shouldn't it?
1min. Need to look in more detail.
OK, I removed them and not on;ly does it still work as intended but is also thread safe now. If you don't think it's a problem then I'll commit it like that.
Yes, since for loop will be on server side these variables are no longer needed to be members and can have local function scope so that garbage collection will free them on return.