refactor: integrate portablemc 5.x with in-memory loading#7
Merged
Conversation
Co-authored-by: PythonChicken123 <101510622+PythonChicken123@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Reviewer's GuideIntegrates optional in-memory loading for the vendored portablemc 5.x native extension into the in-process launcher, adds a reusable .pyd memory loader utility, and threads configuration and logging of the chosen loading mode through the JVM boot glue and CLI entrypoints. Sequence diagram for in-process launcher with portablemc in-memory modesequenceDiagram
actor User
participant main_py as main_py
participant jvm_boot_glue as jvm_boot_glue_py
participant Adapter as PortableMCGameAdapter
participant Env as _ensure_portablemc_environment
participant local_pmc as local_portablemc
participant PydLoader as PydMemoryLoader
User->>main_py: select option 4 or 5
main_py->>main_py: run_inprocess_launcher(memory_resident, portablemc_inmemory)
main_py->>jvm_boot_glue: launch_minecraft(..., portablemc_inmemory)
jvm_boot_glue->>Adapter: PortableMCGameAdapter(...)
jvm_boot_glue->>Adapter: resolve(inmemory=portablemc_inmemory)
Adapter->>Env: _ensure_portablemc_environment(inmemory)
alt inmemory is True
Env->>local_pmc: bootstrap_inmemory(scripts_dir)
local_pmc->>local_pmc: _ensure_pyd(), _scrub_pip_portablemc(), _prepend_vendor()
local_pmc->>local_pmc: _find_pyd_file()
local_pmc->>PydLoader: PydMemoryLoader.load()
PydLoader->>PydLoader: map .pyd via MemoryModule
PydLoader->>PydLoader: call PyInit__portablemc
PydLoader-->>local_pmc: module
local_pmc->>local_pmc: _create_portablemc_package()
local_pmc->>local_pmc: _setup_portablemc_submodule_stubs()
Env-->>Adapter: True, _PMC_INMEMORY_MODE=True
else inmemory is False
Env->>local_pmc: bootstrap(scripts_dir)
Env-->>Adapter: True, _PMC_INMEMORY_MODE=False
end
Adapter->>Adapter: _via_python_api()
Adapter-->>jvm_boot_glue: jvm_flags, classpath, main_class, game_args
jvm_boot_glue->>jvm_boot_glue: log portablemc mode via is_portablemc_inmemory()
jvm_boot_glue-->>main_py: launch JVM and game
main_py-->>User: game runs in-process
Class diagram for new in-memory PYD loading utilitiesclassDiagram
class PydMemoryLoader {
-Path pyd_path
-MemoryModule _memory_module
-ModuleType _module
-str _init_func_name
-bool _loaded
+PydMemoryLoader(pyd_path)
+bool is_loaded
+ModuleType module
-str _infer_module_name()
+bool load()
+bool register_module(name)
+ModuleType get_submodule(submod_name)
+int register_submodules(parent_name, submod_names)
}
class pyd_memory_loader_module {
-dict~str, PydMemoryLoader~ _LOADED_PYDS
+ModuleType load_pyd_inmemory(pyd_path)
+bool is_pyd_loaded(pyd_path)
+PydMemoryLoader get_loaded_pyd(pyd_path)
}
class local_portablemc_module {
-bool _PYD_LOADED_INMEMORY
-object _PYD_MEMORY_MODULE
+Path _find_pyd_file()
+ModuleType _load_pyd_inmemory(pyd_path)
+bool _create_portablemc_package()
+_setup_portablemc_submodule_stubs()
+bool bootstrap_inmemory(scripts_dir)
+bool is_pyd_loaded_inmemory()
+object get_inmemory_module()
}
class jvm_boot_glue_module {
-bool _PMC_BOOTSTRAP_COMPLETE
-bool _PMC_INMEMORY_MODE
+bool _ensure_portablemc_environment(inmemory)
+bool is_portablemc_inmemory()
}
class PortableMCGameAdapter {
+resolve(inmemory) tuple
-tuple _via_python_api()
+static _extract_args(jvm_args, game_args, main_class)
}
class main_module {
+run_inprocess_launcher(memory_resident, portablemc_inmemory)
+main()
}
pyd_memory_loader_module --> PydMemoryLoader : manages
local_portablemc_module --> PydMemoryLoader : uses for in-memory loading
jvm_boot_glue_module --> local_portablemc_module : calls bootstrap_inmemory
PortableMCGameAdapter --> jvm_boot_glue_module : calls _ensure_portablemc_environment
main_module --> jvm_boot_glue_module : calls launch_minecraft
main_module --> PortableMCGameAdapter : indirectly via launch_minecraft
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
PythonChicken123
marked this pull request as ready for review
May 10, 2026 17:46
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The low-level pythonmemorymodule wiring in local_portablemc._load_pyd_inmemory duplicates the new PydMemoryLoader logic in pyd_memory_loader.py; consider refactoring local_portablemc to use PydMemoryLoader so the in-memory loading behavior and error handling are centralized and easier to maintain.
- Several of the section banner comments now contain garbled box-drawing characters (e.g., '══════════════��════════'); it would be good to normalize these strings to avoid encoding artifacts creeping into the codebase.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The low-level pythonmemorymodule wiring in local_portablemc._load_pyd_inmemory duplicates the new PydMemoryLoader logic in pyd_memory_loader.py; consider refactoring local_portablemc to use PydMemoryLoader so the in-memory loading behavior and error handling are centralized and easier to maintain.
- Several of the section banner comments now contain garbled box-drawing characters (e.g., '══════════════��════════'); it would be good to normalize these strings to avoid encoding artifacts creeping into the codebase.
## Individual Comments
### Comment 1
<location path="scripts/local_portablemc.py" line_range="268-277" />
<code_context>
+ mem_mod = MemoryModule(data=pyd_data, debug=False)
</code_context>
<issue_to_address>
**issue (bug_risk):** Keep a strong reference to MemoryModule to avoid underlying mapping being GC'd while the module is in use.
Because `MemoryModule` is only assigned to the local `mem_mod`, it can be garbage-collected after `_load_pyd_inmemory` returns, potentially unmapping the PE image while `_portablemc` is still in use and causing crashes. Please keep a strong reference for at least as long as the module is loaded (e.g., via a module-level variable like `_PYD_MEMORY_HANDLE` or an attribute on the module).
</issue_to_address>
### Comment 2
<location path="scripts/local_portablemc.py" line_range="240-249" />
<code_context>
+ return None
+
+
+def _load_pyd_inmemory(pyd_path: Path) -> types.ModuleType | None:
+ """
+ Load a .pyd file into memory using pythonmemorymodule.
+
+ Returns the loaded module object, or None on failure.
+ This function:
+ 1. Reads the .pyd binary into memory
+ 2. Uses pythonmemorymodule.MemoryModule to map it
+ 3. Creates a module object and initializes it via PyInit_*
+ """
+ global _PYD_LOADED_INMEMORY, _PYD_MEMORY_MODULE
+
+ if _PYD_LOADED_INMEMORY and _PYD_MEMORY_MODULE is not None:
+ log.debug("_portablemc pyd already loaded in-memory")
+ return _PYD_MEMORY_MODULE
+
+ try:
</code_context>
<issue_to_address>
**suggestion:** Consider reusing the generic PydMemoryLoader instead of duplicating in-memory .pyd loading logic.
This function reimplements the same flow as `PydMemoryLoader` (reading the .pyd, creating `MemoryModule`, invoking `PyInit_*`, and registering into `sys.modules`). To avoid duplication and drift, consider having `_load_pyd_inmemory` call into `PydMemoryLoader` and then perform any portablemc-specific registration, so fixes for error handling or platform-specific behavior stay centralized.
Suggested implementation:
```python
def _load_pyd_inmemory(pyd_path: Path) -> types.ModuleType | None:
"""
Load a .pyd file into memory using the generic PydMemoryLoader.
Returns the loaded module object, or None on failure.
This function delegates the low-level in-memory loading to PydMemoryLoader
and only handles portablemc-specific caching of the loaded module.
"""
global _PYD_LOADED_INMEMORY, _PYD_MEMORY_MODULE
if _PYD_LOADED_INMEMORY and _PYD_MEMORY_MODULE is not None:
log.debug("_portablemc pyd already loaded in-memory")
return _PYD_MEMORY_MODULE
try:
from some_module import PydMemoryLoader # adjust import to actual location
log.info("Loading _portablemc.pyd in-memory from: %s", pyd_path)
loader = PydMemoryLoader(pyd_path)
module = loader.load()
except Exception:
log.exception("Failed to load _portablemc.pyd in-memory from %s", pyd_path)
return None
_PYD_LOADED_INMEMORY = True
_PYD_MEMORY_MODULE = module
return module
```
1. Replace `from some_module import PydMemoryLoader` with the correct import path for `PydMemoryLoader` as used elsewhere in your codebase.
2. Remove any remaining legacy in-memory loading logic that may still exist after this snippet (e.g., direct `pythonmemorymodule.MemoryModule` usage, manual `PyInit_*` invocation, or `sys.modules` registration) so that `_load_pyd_inmemory` only delegates to `PydMemoryLoader` and performs caching.
3. Ensure `PydMemoryLoader.load()` returns the initialized module object and handles platform-specific behaviors and error handling consistently with its other call sites.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+268
to
+277
| mem_mod = MemoryModule(data=pyd_data, debug=False) | ||
|
|
||
| # Get the PyInit function address | ||
| # The function name is PyInit_<module_name> for Python 3 extensions | ||
| init_func_name = b"PyInit__portablemc" | ||
| init_func_addr = mem_mod.get_proc_addr("PyInit__portablemc") | ||
|
|
||
| if not init_func_addr: | ||
| log.error("Could not find PyInit__portablemc in pyd") | ||
| return None |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Keep a strong reference to MemoryModule to avoid underlying mapping being GC'd while the module is in use.
Because MemoryModule is only assigned to the local mem_mod, it can be garbage-collected after _load_pyd_inmemory returns, potentially unmapping the PE image while _portablemc is still in use and causing crashes. Please keep a strong reference for at least as long as the module is loaded (e.g., via a module-level variable like _PYD_MEMORY_HANDLE or an attribute on the module).
Comment on lines
+240
to
+249
| def _load_pyd_inmemory(pyd_path: Path) -> types.ModuleType | None: | ||
| """ | ||
| Load a .pyd file into memory using pythonmemorymodule. | ||
|
|
||
| Returns the loaded module object, or None on failure. | ||
| This function: | ||
| 1. Reads the .pyd binary into memory | ||
| 2. Uses pythonmemorymodule.MemoryModule to map it | ||
| 3. Creates a module object and initializes it via PyInit_* | ||
| """ |
Contributor
There was a problem hiding this comment.
suggestion: Consider reusing the generic PydMemoryLoader instead of duplicating in-memory .pyd loading logic.
This function reimplements the same flow as PydMemoryLoader (reading the .pyd, creating MemoryModule, invoking PyInit_*, and registering into sys.modules). To avoid duplication and drift, consider having _load_pyd_inmemory call into PydMemoryLoader and then perform any portablemc-specific registration, so fixes for error handling or platform-specific behavior stay centralized.
Suggested implementation:
def _load_pyd_inmemory(pyd_path: Path) -> types.ModuleType | None:
"""
Load a .pyd file into memory using the generic PydMemoryLoader.
Returns the loaded module object, or None on failure.
This function delegates the low-level in-memory loading to PydMemoryLoader
and only handles portablemc-specific caching of the loaded module.
"""
global _PYD_LOADED_INMEMORY, _PYD_MEMORY_MODULE
if _PYD_LOADED_INMEMORY and _PYD_MEMORY_MODULE is not None:
log.debug("_portablemc pyd already loaded in-memory")
return _PYD_MEMORY_MODULE
try:
from some_module import PydMemoryLoader # adjust import to actual location
log.info("Loading _portablemc.pyd in-memory from: %s", pyd_path)
loader = PydMemoryLoader(pyd_path)
module = loader.load()
except Exception:
log.exception("Failed to load _portablemc.pyd in-memory from %s", pyd_path)
return None
_PYD_LOADED_INMEMORY = True
_PYD_MEMORY_MODULE = module
return module- Replace
from some_module import PydMemoryLoaderwith the correct import path forPydMemoryLoaderas used elsewhere in your codebase. - Remove any remaining legacy in-memory loading logic that may still exist after this snippet (e.g., direct
pythonmemorymodule.MemoryModuleusage, manualPyInit_*invocation, orsys.modulesregistration) so that_load_pyd_inmemoryonly delegates toPydMemoryLoaderand performs caching. - Ensure
PydMemoryLoader.load()returns the initialized module object and handles platform-specific behaviors and error handling consistently with its other call sites.
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
Add this suggestion to a batch that can be applied as a single commit.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.Add this suggestion to a batch that can be applied as a single commit.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.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Add support for loading the portablemc 5.x native extension (.pyd) directly into memory and integrate this mode into the in-process launcher flow for environments with strict executable policies.
New Features:
Enhancements:
Documentation: