Replies: 1 comment
-
I should add that using the shared Lua state as a shared data store is far more robust. Data stored there (provided it's not stored as lightuserdata which is generally not the case) is owned by the Lua interpreter, so there is far less risk of a dangled pointer (there is still risk since some data types are in fact stored as lightuserdata). If I'm not mistaken, we are using the shared Lua state for this purpose now in several places. Removing this functionality still leaves the option of using shared Lua state to accomplish the same goal in a generally safer, albeit somewhat slower, manner. |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
This functionality allows for "anything" to associate an opaque pointer with a named key in a map within
Core
, and for "anything" to retrieve that opaque pointer by its named key.As far as I can tell, it was added in 2011 by peterix, and then never used for anything.
The problem I have with this is that there's nothing that ensure that the stored pointer never gets dangled. If some tool registers a pointer to an object under some key, and then destroys that object, the pointer will remain in
Core
s map, pointing at invalid memory. It could even point to unmapped memory, if the object was a static object in a plug-in DLL that has been subsequently unloaded and thus unmapped; thus, even attempting to deference the retrieved pointer could cause a crash.The problem is that while the pointers are stored in
Core
, which is guaranteed to exist basically for as long as DFHack is active, the pointers themselves have no specified lifetime and could disappear entirely without warning.Given the fundamental unsafeness of the mechanic and the fact that it is entirely unused, I recommend removing it from
Core
. If we want a mechanic like this, we should make it robust, and this implementation just isn't.Beta Was this translation helpful? Give feedback.
All reactions