search

OpenCollarTeam / OpenCollar

Other
123 stars 128 forks source link

Stack-heap collision in #RLV browsing #46

Closed SatomiAhn closed 6 years ago

SatomiAhn commented 6 years ago

What version of OpenCollar are you using? 6.x and 7.0beta

What behavior did you expect? Stack-heap collisions should never happen whatever we do.

What behavior did you see instead? When I browse a #RLV folder with many folders, using oc_folder script, this script may crash on stack-heap collision.

What steps does someone need to take to reproduce the problem? Prepare a large folder "TestFolder" of wearable subfolders under #RLV. Call the command "#rlv TestFolder" and observe. How large should TestFolder be ? That remains to determine!

Additional details: I asked the script to display its free memory on startup. It happens that merely 9854 bytes are available, which is totally not enough to do the job this script is supposed to do (to put things into perspective, a RLV query answer may be 4k chars, and some room will still be needed to analyze it).

Trying to investigate why #RLV browsing does not work when it used to, I remembered the discussion about the FailSafe() (now CheckPerms()) function in issue #44. So I deleted this function and all reference to it. Free memory on startup is now 12926 bytes (~3k more than before), which happens to be sufficient for my test folder (but still may not be for larger ones, although I suspect I could find other unnecessary things in this script).

Conclusion : this script (and maybe others) need a good clean up from any stuff that is not absolutely necessary for their core functionality.

Of course, tests could also be added in order not to overflow memory and display an error instead of crashing when memory is insufficient.

nirea commented 6 years ago

I run into this too if I browse into the wrong folder. I have no problem putting the script on a diet, including by putting PermsCheck into just one script, though that's still just reducing the problem rather than solving it.

The best solution, I think, would be pagination in the RLV interface. If you could pass offset/limit parameters to the viewer (e.g. "Only give me 9 folders, and start at the 10th"), then we could support any size of inventory.

SatomiAhn commented 6 years ago

If the solution is changing upstream stuff, we could as well ask for a less ludicrously low memory limit for scripts... But fundamentally yes, we need some control on the size of data we want to receive and handle. I believe this was already requested to RLV or RLVa in the past.

As a side note: the size of a chat message is bounded (1024 bytes for llSay. Can viewers send longer messages?), so it should be possible to guarantee that memory usage remains bounded in all cases (and that this bound stays below 64kB). But this would mean a serious diet for oc_folders script.

nirea commented 6 years ago

PermsCheck has been centralized to oc_sys as of https://github.com/OpenCollarTeam/OpenCollar/commit/c17c818d9279e92d4baf6814d512d4ccc80cc97f, which also removes all unused variables and functions identified by make lint. I think that's as good as we can do right now.