{{ message }}
attempt at a proper fix for the BiVariableMap memory leak#9452
Draft
matthias-fratz-bsz wants to merge 2 commits into
Draft
attempt at a proper fix for the BiVariableMap memory leak#9452matthias-fratz-bsz wants to merge 2 commits into
matthias-fratz-bsz wants to merge 2 commits into
Conversation
Contributor
Sorry, something went wrong.
matthias-fratz-bsz
force-pushed
the
bvm-refactoring
branch
from
2e75dfb to
fda8269
Compare
May 20, 2026 14:04
This essentially ties each BiVariableMap instance to a particular thread in the context-per-thread providers. Using it from a different thread then behaves consistently as long as both threads aren't running concurrently. (Using a BiVariableMap concurrently from multiple threads is as race-y as before.) This commit also moves some global runtime logic into a LocalContext subclass called GlobalContext and refactors the various LocalContextProvider classes to delegate almost everything to LocalContext.
This introduces some minor behavior changes: - The "java type" of variables set via the RedBridge API is no longer stored. Thus a variable that has been set to `String[]` will now come back as a `RubyArray` object which can be used as a `List<String>`. The undocumented previous behavior was to convert the value back to the same type it was originally. - BiVariableMap no longer holds a copy of all variables; it only actually stores local varibles. All other types of variable are simply retrieved from either the runtime's global state, or from the given receiver object, if and when the variable is accessed. - As a corollary, variables can now change value between the evaluation and the moment when they are accessed via BiVariableMap, if some other threads modifies them concurrently. This mostly affects LocalContextProvider that use the global runtime, i.e. ConcurrentLocalContextProvider and obviously SingletonLocalContextProvider. Such usage has always been race-y, but the detail on what constitutes the critical section has changed.
matthias-fratz-bsz
force-pushed
the
bvm-refactoring
branch
from
fda8269 to
306ee51
Compare
May 20, 2026 14:09
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.
This is an attempt at refactoring the
BiVariableMapinterface so it no longer keeps a copy of every single Ruby variable. It does still have to store all local variables, but for everything else it can retrieve them from either the specified receiver object or the Ruby runtime's global state.This means instance varibles are no longer stored in
BiVariableMap, thus they can no longer cause the #8527 memory leak. It should also improve performance somewhat because it avoids a lot of copying before and after every evaluation.It does however introduce 2 minor behavior changes:
String[]will now come back as aRubyArrayobject (which can be used as aList<String>, but notString[]) when retrieved after evaluation. This is undocumented behavior, but it's a change from the previous behavior where reading the variable after evaluation would always try to convert its value back to the same type that was used for setting it.LocalContextProviders that use the global runtime, i.e.ConcurrentLocalContextProviderand obviouslySingletonLocalContextProvider, this means that other threads can change the value of a global variable, constant, class variable, etc. between the evaluation and when the variable is read. As far as I can tell, evaluation and retrieval of (global etc.) variables has never been an atomic operation to begin with, but the timing window for getting race-y behavior is now somewhat larger.