Skip to content

Commit de2e06c

Browse files
committed
fix: handling of weakrefs and non copieable objects
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
1 parent 35429fd commit de2e06c

8 files changed

Lines changed: 263 additions & 11 deletions

File tree

.coveragerc

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
[run]
2+
omit =
3+
*__init__.py
4+
*/templates/*
5+
6+
[report]
7+
omit =
8+
*__init__.py
9+
*/templates/*

cpex/framework/__init__.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
# First-Party
2020
from cpex.framework.base import Plugin
2121
from cpex.framework.decorator import hook
22-
from cpex.framework.errors import PluginError, PluginViolationError
22+
from cpex.framework.errors import PluginError, PluginFrameworkError, PluginViolationError
2323
from cpex.framework.external.mcp.server import ExternalPluginServer
2424
from cpex.framework.hooks.agents import (
2525
AgentHookType,
@@ -163,6 +163,7 @@ def get_plugin_manager(
163163
"PluginContextTable",
164164
"PluginError",
165165
"PluginErrorModel",
166+
"PluginFrameworkError",
166167
"PluginLoader",
167168
"PluginManager",
168169
"PluginMode",

cpex/framework/errors.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,32 @@
1313
from cpex.framework.models import PluginErrorModel, PluginViolation
1414

1515

16+
class PluginFrameworkError(Exception):
17+
"""An error originating from the plugin framework itself (not from a plugin).
18+
19+
Raised when an internal framework operation fails — for example, when
20+
payload isolation cannot deep-copy a value.
21+
22+
Attributes:
23+
message (str): Description of the framework error.
24+
"""
25+
26+
def __init__(self, message: str):
27+
"""Initialize a plugin framework error.
28+
29+
Args:
30+
message: Description of what went wrong.
31+
32+
Examples:
33+
>>> from cpex.framework.errors import PluginFrameworkError
34+
>>> err = PluginFrameworkError("cannot isolate payload")
35+
>>> str(err)
36+
'cannot isolate payload'
37+
"""
38+
self.message = message
39+
super().__init__(self.message)
40+
41+
1642
class PluginViolationError(Exception):
1743
"""A plugin violation error.
1844

cpex/framework/manager.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929

3030
# Standard
3131
import asyncio
32-
import copy
3332
import logging
3433
import threading
3534
from typing import Any, Literal, Optional, Union
@@ -43,7 +42,7 @@
4342
from cpex.framework.hooks.policies import DefaultHookPolicy, HookPayloadPolicy, apply_policy
4443
from cpex.framework.loader.config import ConfigLoader
4544
from cpex.framework.loader.plugin import PluginLoader
46-
from cpex.framework.memory import copyonwrite, wrap_payload_for_isolation
45+
from cpex.framework.memory import _safe_deepcopy, copyonwrite, wrap_payload_for_isolation
4746
from cpex.framework.models import (
4847
Config,
4948
GlobalContext,
@@ -496,7 +495,7 @@ def _isolate_payload(
496495
return effective_payload
497496
if isinstance(effective_payload, BaseModel):
498497
return wrap_payload_for_isolation(effective_payload)
499-
return copy.deepcopy(effective_payload)
498+
return _safe_deepcopy(effective_payload)
500499

501500
def _build_halt_result(
502501
self,
@@ -558,7 +557,7 @@ def _fire_and_forget_tasks(
558557
# Already scheduled — skip to avoid double-scheduling
559558
continue
560559
task_input = (
561-
wrap_payload_for_isolation(payload) if isinstance(payload, BaseModel) else copy.deepcopy(payload)
560+
wrap_payload_for_isolation(payload) if isinstance(payload, BaseModel) else _safe_deepcopy(payload)
562561
)
563562
tmp_gc = GlobalContext(
564563
request_id=global_context.request_id,

cpex/framework/memory.py

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,18 @@
1212

1313
# Standard
1414
import copy
15+
import logging
16+
import weakref
1517
from typing import Any, Iterator, Optional, TypeVar
1618

1719
# Third-Party
1820
from pydantic import BaseModel, RootModel
1921

22+
# First-Party
23+
from cpex.framework.errors import PluginFrameworkError
24+
2025
T = TypeVar("T")
26+
logger = logging.getLogger(__name__)
2127

2228

2329
class CopyOnWriteDict(dict):
@@ -517,6 +523,21 @@ def copyonwrite(o: T) -> T:
517523
_PRIMITIVE_TYPES = (str, int, float, bool, bytes, type(None))
518524

519525

526+
def _safe_deepcopy(value: Any) -> Any:
527+
"""Deep-copy *value* with diagnostic context on failure.
528+
529+
Wraps :func:`copy.deepcopy` so that failures surface the concrete type and
530+
a truncated repr, making it much easier to identify which field or object
531+
caused the issue (e.g. file handles, C extensions, locks).
532+
"""
533+
try:
534+
return copy.deepcopy(value)
535+
except Exception as e:
536+
raise PluginFrameworkError(
537+
f"Cannot deep-copy value of type {type(value).__qualname__} (repr={repr(value)!s:.200}): {e}"
538+
) from e
539+
540+
520541
def _wrap_value(value: Any) -> Any:
521542
"""Wrap a single value with the appropriate CoW wrapper.
522543
@@ -536,16 +557,20 @@ def _wrap_value(value: Any) -> Any:
536557
elif isinstance(root, list):
537558
wrapped_root = CopyOnWriteList(root)
538559
else:
539-
wrapped_root = copy.deepcopy(root)
560+
wrapped_root = _safe_deepcopy(root)
540561
return value.model_construct(root=wrapped_root)
541562
if isinstance(value, BaseModel):
542563
return wrap_payload_for_isolation(value)
543564
if isinstance(value, dict):
544565
return CopyOnWriteDict(value)
545566
if isinstance(value, list):
546567
return CopyOnWriteList(value)
568+
# Weak-reference proxies wrap live objects that must not be
569+
# copied — pass them through as-is so plugins see the same proxy.
570+
if isinstance(value, (weakref.ProxyType, weakref.CallableProxyType)):
571+
return value
547572
# Other mutable types — fallback to deep copy
548-
return copy.deepcopy(value)
573+
return _safe_deepcopy(value)
549574

550575

551576
def wrap_payload_for_isolation(payload: BaseModel) -> BaseModel:
@@ -570,7 +595,7 @@ def wrap_payload_for_isolation(payload: BaseModel) -> BaseModel:
570595
elif isinstance(root, list):
571596
wrapped_root = CopyOnWriteList(root)
572597
else:
573-
wrapped_root = copy.deepcopy(root)
598+
wrapped_root = _safe_deepcopy(root)
574599
return payload.model_construct(root=wrapped_root)
575600

576601
updates = {}

tests/pytest.ini

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,3 @@ pythonpath = .
1212
filterwarnings =
1313
ignore::DeprecationWarning:pydantic.*
1414
ignore::DeprecationWarning:pythonjsonlogger.*
15-
16-
[coverage:run]
17-
omit = *__init__.py

tests/unit/cpex/framework/loader/test_plugin_loader.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ async def test_plugin_loader_shutdown_with_existing_types():
193193
"""Test shutdown clears existing plugin types."""
194194
config = ConfigLoader.load_config(config="./tests/unit/cpex/fixtures/configs/valid_single_plugin.yaml")
195195
loader = PluginLoader()
196+
loader.append_to_search_path(config.plugin_dirs)
196197

197198
# Load a plugin to populate _plugin_types
198199
plugin = await loader.load_and_instantiate_plugin(config.plugins[0])
@@ -224,6 +225,9 @@ async def test_plugin_loader_registration_branch_coverage():
224225
config={"words": [{"search": "test", "replace": "example"}]},
225226
)
226227

228+
# Add plugin dirs to search path so importlib can resolve the kind
229+
loader.append_to_search_path(["tests/unit/cpex/fixtures"])
230+
227231
# First load - should register the plugin type (lines 85-87)
228232
assert config.kind not in loader._plugin_types # Verify it's not registered yet
229233
plugin1 = await loader.load_and_instantiate_plugin(config)

0 commit comments

Comments
 (0)