The Variables thread (previously Environment thread) preserves a copy of the previously seen bindings in the current_bindings field. These contain binding names as R symbols (permanently interned in the session and thus safe) and binding values represented by SEXP objects.
The current bindings are stored on the Variables thread without taking any measures to protect the values it points to. This means that if the environment holding the bindings (only the global env currently) is updated at some point, these might point to dangling pointers that could cause a crash when dereferenced.
These bindings are currently only consulted in two method: refresh() and update(). I reviewed them and they currently do not seem problematic. The former is safe as it first updates the bindings to their current state. The latter might be inspecting dangling pointers but there is no risk of crash because only the pointer address is inspected to detect outdated bindings.
However I think we should still protect these bindings and prevent garbage collection to be defensive against future changes that might inspect them in unsafe ways. Also this will rule out the possibility of incorrect detection of new or changed bindings when a new binding points to a previously used SEXP address, which could happen from time to time via R's small objects pool.
lionel-
commented
1 year ago
The Variables thread (previously Environment thread) preserves a copy of the previously seen bindings in the
current_bindingsfield. These contain binding names as R symbols (permanently interned in the session and thus safe) and binding values represented bySEXPobjects.The current bindings are stored on the Variables thread without taking any measures to protect the values it points to. This means that if the environment holding the bindings (only the global env currently) is updated at some point, these might point to dangling pointers that could cause a crash when dereferenced.
These bindings are currently only consulted in two method:
refresh()andupdate(). I reviewed them and they currently do not seem problematic. The former is safe as it first updates the bindings to their current state. The latter might be inspecting dangling pointers but there is no risk of crash because only the pointer address is inspected to detect outdated bindings.However I think we should still protect these bindings and prevent garbage collection to be defensive against future changes that might inspect them in unsafe ways. Also this will rule out the possibility of incorrect detection of new or changed bindings when a new binding points to a previously used
SEXPaddress, which could happen from time to time via R's small objects pool.