From db7bd69a75c355364c0903f008a61a20bf0109a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Lehfeld?= <54720674+rlehfeld@users.noreply.github.com> Date: Sat, 27 Apr 2024 15:06:28 +0200 Subject: [PATCH 01/11] commit support for objects and classes without __class__ property --- rpyc/lib/__init__.py | 79 ++++++++++++++++++++++++++++---------------- 1 file changed, 50 insertions(+), 29 deletions(-) diff --git a/rpyc/lib/__init__.py b/rpyc/lib/__init__.py index 88f1a59e..03c16758 100644 --- a/rpyc/lib/__init__.py +++ b/rpyc/lib/__init__.py @@ -170,44 +170,65 @@ def exp_backoff(collision): def get_id_pack(obj): - """introspects the given "local" object, returns id_pack as expected by BaseNetref + """introspects the given "local" object, returns id_pack as expected by + BaseNetref - The given object is "local" in the sense that it is from the local cache. Any object in the local cache exists - in the current address space or is a netref. A netref in the local cache could be from a chained-connection. - To handle type related behavior properly, the attribute `__class__` is a descriptor for netrefs. + The given object is "local" in the sense that it is from the local + cache. Any object in the local cache exists in the current address + space or is a netref. A netref in the local cache could be from a + chained-connection. To handle type related behavior properly, the + attribute `__class__` is a descriptor for netrefs. - So, check thy assumptions regarding the given object when creating `id_pack`. + So, check thy assumptions regarding the given object when creating + `id_pack`. """ - if hasattr(obj, '____id_pack__'): - # netrefs are handled first since __class__ is a descriptor - return obj.____id_pack__ - elif inspect.ismodule(obj) or getattr(obj, '__name__', None) == 'module': - # TODO: not sure about this, need to enumerate cases in units + undef = object() + name_pack = getattr(obj, '____id_pack__', undef) + if name_pack is not undef: + return name_pack + + obj_name = getattr(obj, '__name__', None) + if (inspect.ismodule(obj) or obj_name == 'module'): if isinstance(obj, type): # module obj_cls = type(obj) - name_pack = '{0}.{1}'.format(obj_cls.__module__, obj_cls.__name__) + name_pack = ( + f'{obj_cls.__module__}.{obj_cls.__name__}' + ) return (name_pack, id(type(obj)), id(obj)) + + if inspect.ismodule(obj) and obj_name != 'module': + if obj_name in sys.modules: + name_pack = obj_name + else: + obj_cls = getattr(obj, '__class__', type(obj)) + name_pack = ( + f'{obj_cls.__module__}.{obj_name}' + ) + elif inspect.ismodule(obj): + name_pack = ( + f'{obj.__module__}.{obj_name}' + ) else: - if inspect.ismodule(obj) and obj.__name__ != 'module': - if obj.__name__ in sys.modules: - name_pack = obj.__name__ - else: - name_pack = '{0}.{1}'.format(obj.__class__.__module__, obj.__name__) - elif inspect.ismodule(obj): - name_pack = '{0}.{1}'.format(obj.__module__, obj.__name__) - print(name_pack) - elif hasattr(obj, '__module__'): - name_pack = '{0}.{1}'.format(obj.__module__, obj.__name__) + obj_module = getattr(obj, '__module__', undef) + if obj_module is not undef: + name_pack = ( + f'{obj.__module__}.{obj_name}' + ) else: - obj_cls = type(obj) - name_pack = '{0}'.format(obj.__name__) - return (name_pack, id(type(obj)), id(obj)) - elif not inspect.isclass(obj): - name_pack = '{0}.{1}'.format(obj.__class__.__module__, obj.__class__.__name__) + name_pack = obj_name return (name_pack, id(type(obj)), id(obj)) - else: - name_pack = '{0}.{1}'.format(obj.__module__, obj.__name__) - return (name_pack, id(obj), 0) + + if not inspect.isclass(obj): + obj_cls = getattr(obj, '__class__', type(obj)) + name_pack = ( + f'{obj_cls.__module__}.{obj_cls.__name__}' + ) + return (name_pack, id(type(obj)), id(obj)) + + name_pack = ( + f'{obj.__module__}.{obj_name}' + ) + return (name_pack, id(obj), 0) def get_methods(obj_attrs, obj): From 599b9051779a8d62ead1999cc754ae630f115b90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Lehfeld?= <54720674+rlehfeld@users.noreply.github.com> Date: Sat, 27 Apr 2024 15:09:50 +0200 Subject: [PATCH 02/11] create dummy commit --- rpyc/lib/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/rpyc/lib/__init__.py b/rpyc/lib/__init__.py index 03c16758..c578cbe6 100644 --- a/rpyc/lib/__init__.py +++ b/rpyc/lib/__init__.py @@ -11,6 +11,7 @@ from rpyc.lib.compat import maxint # noqa: F401 + SPAWN_THREAD_PREFIX = 'RpycSpawnThread' From c8f8a28b03eb29a1b51fc728ee12935f916489f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Lehfeld?= <54720674+rlehfeld@users.noreply.github.com> Date: Sat, 27 Apr 2024 15:10:04 +0200 Subject: [PATCH 03/11] undo dummy commit --- rpyc/lib/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/rpyc/lib/__init__.py b/rpyc/lib/__init__.py index c578cbe6..03c16758 100644 --- a/rpyc/lib/__init__.py +++ b/rpyc/lib/__init__.py @@ -11,7 +11,6 @@ from rpyc.lib.compat import maxint # noqa: F401 - SPAWN_THREAD_PREFIX = 'RpycSpawnThread' From 151d1e326fc006d931c38ab793101d4ec1db5011 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Lehfeld?= <54720674+rlehfeld@users.noreply.github.com> Date: Mon, 29 Apr 2024 22:39:55 +0200 Subject: [PATCH 04/11] do not use hasattr but instead insepct.isroutine --- rpyc/lib/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rpyc/lib/__init__.py b/rpyc/lib/__init__.py index 03c16758..93aff1b0 100644 --- a/rpyc/lib/__init__.py +++ b/rpyc/lib/__init__.py @@ -250,6 +250,6 @@ def get_methods(obj_attrs, obj): for basecls in mros: attrs.update(basecls.__dict__) for name, attr in attrs.items(): - if name not in obj_attrs and hasattr(attr, "__call__"): + if name not in obj_attrs and inspect.isroutine(attr): methods[name] = inspect.getdoc(attr) return methods.items() From ac283a9a90b9df5b1fe2c28e294f544f6c38c014 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Lehfeld?= <54720674+rlehfeld@users.noreply.github.com> Date: Fri, 14 Jun 2024 02:01:54 +0200 Subject: [PATCH 05/11] fix race when the garbage collector jumps in the middle between the check if element is in the proxy_cache and getting the element from the cache --- rpyc/core/protocol.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rpyc/core/protocol.py b/rpyc/core/protocol.py index d7a90f61..d31b2eca 100644 --- a/rpyc/core/protocol.py +++ b/rpyc/core/protocol.py @@ -336,8 +336,8 @@ def _unbox(self, package): # boxing return self._local_objects[value] if label == consts.LABEL_REMOTE_REF: id_pack = (str(value[0]), value[1], value[2]) # so value is a id_pack - if id_pack in self._proxy_cache: - proxy = self._proxy_cache[id_pack] + proxy = self._proxy_cache.get(id_pack) + if proxy is not None: proxy.____refcount__ += 1 # if cached then remote incremented refcount, so sync refcount else: proxy = self._netref_factory(id_pack) From 808676e55c2167353dd53eeffb7d783fd2fd32e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Lehfeld?= <54720674+rlehfeld@users.noreply.github.com> Date: Fri, 14 Jun 2024 02:29:23 +0200 Subject: [PATCH 06/11] revert change --- rpyc/core/protocol.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rpyc/core/protocol.py b/rpyc/core/protocol.py index d31b2eca..40e364e9 100644 --- a/rpyc/core/protocol.py +++ b/rpyc/core/protocol.py @@ -337,7 +337,8 @@ def _unbox(self, package): # boxing if label == consts.LABEL_REMOTE_REF: id_pack = (str(value[0]), value[1], value[2]) # so value is a id_pack proxy = self._proxy_cache.get(id_pack) - if proxy is not None: + if id_pack in self._proxy_cache: + proxy = self._proxy_cache[id_pack] proxy.____refcount__ += 1 # if cached then remote incremented refcount, so sync refcount else: proxy = self._netref_factory(id_pack) From e8ae2ba967df156fdfaea9d15f425257c6f4216f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Lehfeld?= <54720674+rlehfeld@users.noreply.github.com> Date: Fri, 14 Jun 2024 02:38:17 +0200 Subject: [PATCH 07/11] looks like None is also stored. Use temporary instance so nonexistent situation can be detected --- rpyc/core/protocol.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rpyc/core/protocol.py b/rpyc/core/protocol.py index 40e364e9..b5cc2f92 100644 --- a/rpyc/core/protocol.py +++ b/rpyc/core/protocol.py @@ -336,8 +336,9 @@ def _unbox(self, package): # boxing return self._local_objects[value] if label == consts.LABEL_REMOTE_REF: id_pack = (str(value[0]), value[1], value[2]) # so value is a id_pack - proxy = self._proxy_cache.get(id_pack) - if id_pack in self._proxy_cache: + nonexistent = object() + proxy = self._proxy_cache.get(id_pack, nonexistent) + if proxy is not nonexistent: proxy = self._proxy_cache[id_pack] proxy.____refcount__ += 1 # if cached then remote incremented refcount, so sync refcount else: From 20f33923e04d004f6b977ef208a29f0b4e92c078 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Lehfeld?= <54720674+rlehfeld@users.noreply.github.com> Date: Fri, 14 Jun 2024 03:00:01 +0200 Subject: [PATCH 08/11] still does not work --- rpyc/core/protocol.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/rpyc/core/protocol.py b/rpyc/core/protocol.py index b5cc2f92..d7a90f61 100644 --- a/rpyc/core/protocol.py +++ b/rpyc/core/protocol.py @@ -336,9 +336,7 @@ def _unbox(self, package): # boxing return self._local_objects[value] if label == consts.LABEL_REMOTE_REF: id_pack = (str(value[0]), value[1], value[2]) # so value is a id_pack - nonexistent = object() - proxy = self._proxy_cache.get(id_pack, nonexistent) - if proxy is not nonexistent: + if id_pack in self._proxy_cache: proxy = self._proxy_cache[id_pack] proxy.____refcount__ += 1 # if cached then remote incremented refcount, so sync refcount else: From c2365ff291399a8dd3dc38328ad8c5eecf1ea237 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Lehfeld?= <54720674+rlehfeld@users.noreply.github.com> Date: Fri, 14 Jun 2024 03:47:50 +0200 Subject: [PATCH 09/11] try again proper fix. Did it actually work when looking at the result from the old actions? --- rpyc/core/protocol.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rpyc/core/protocol.py b/rpyc/core/protocol.py index d7a90f61..d31b2eca 100644 --- a/rpyc/core/protocol.py +++ b/rpyc/core/protocol.py @@ -336,8 +336,8 @@ def _unbox(self, package): # boxing return self._local_objects[value] if label == consts.LABEL_REMOTE_REF: id_pack = (str(value[0]), value[1], value[2]) # so value is a id_pack - if id_pack in self._proxy_cache: - proxy = self._proxy_cache[id_pack] + proxy = self._proxy_cache.get(id_pack) + if proxy is not None: proxy.____refcount__ += 1 # if cached then remote incremented refcount, so sync refcount else: proxy = self._netref_factory(id_pack) From 827dd48b826b5e07c17efd07ac1617946705dec3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Lehfeld?= <54720674+rlehfeld@users.noreply.github.com> Date: Sat, 15 Jun 2024 18:26:02 +0200 Subject: [PATCH 10/11] fix another possible race related to WeakValueDict --- rpyc/lib/colls.py | 1 + 1 file changed, 1 insertion(+) diff --git a/rpyc/lib/colls.py b/rpyc/lib/colls.py index e3b2be98..5cae9a06 100644 --- a/rpyc/lib/colls.py +++ b/rpyc/lib/colls.py @@ -37,6 +37,7 @@ def __getitem__(self, key): obj = self._dict[key]() if obj is None: raise KeyError(key) + self[key] = obj return obj def __setitem__(self, key, value): From 6c6e60e2c147c94bd552bdaa8a770e394bb764ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Lehfeld?= <54720674+rlehfeld@users.noreply.github.com> Date: Sat, 15 Jun 2024 19:23:13 +0200 Subject: [PATCH 11/11] when taking ownership of stdin and stdout in PipeStream, the stdin and stdout must be replaced by devnull as on one hand the code executed could write to it but also we will close the channels and thus cause exit code failures --- rpyc/core/stream.py | 10 ++++++++-- rpyc/lib/colls.py | 1 - 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/rpyc/core/stream.py b/rpyc/core/stream.py index 859faafb..fb8baaa4 100644 --- a/rpyc/core/stream.py +++ b/rpyc/core/stream.py @@ -326,7 +326,10 @@ def from_std(cls): :returns: a :class:`PipeStream` instance """ - return cls(sys.stdin, sys.stdout) + pipestream = cls(sys.stdin, sys.stdout) + sys.stdin = os.open(os.devnull, os.O_RDWR) + sys.stdout = sys.stdin + return pipestream @classmethod def create_pair(cls): @@ -405,7 +408,10 @@ def __init__(self, incoming, outgoing): @classmethod def from_std(cls): - return cls(sys.stdin, sys.stdout) + pipestream = cls(sys.stdin, sys.stdout) + sys.stdin = os.open(os.devnull, os.O_RDWR) + sys.stdout = sys.stdin + return pipestream @classmethod def create_pair(cls): diff --git a/rpyc/lib/colls.py b/rpyc/lib/colls.py index 5cae9a06..e3b2be98 100644 --- a/rpyc/lib/colls.py +++ b/rpyc/lib/colls.py @@ -37,7 +37,6 @@ def __getitem__(self, key): obj = self._dict[key]() if obj is None: raise KeyError(key) - self[key] = obj return obj def __setitem__(self, key, value):