Skip to content

Commit

Permalink
Cache the return message instead of the value
Browse files Browse the repository at this point in the history
Thanks to @richardsheridan for pointing out the limitations of using
*any* kind of value as the result-cached-flag and how it might cause
problems for anyone returning pickled blob-data. This changes the
`Portal` internal result value tracking to stash the full message from
which the value can be retrieved by any `Portal.result()` caller.
The internal change is that `Portal._return_once()` now returns a tuple
of the message *and* its value.
  • Loading branch information
goodboy committed Nov 29, 2021
1 parent 83da92d commit 0e7234a
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 16 deletions.
5 changes: 3 additions & 2 deletions newsfragments/264.bug.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ Fix ``Portal.run_in_actor()`` returns ``None`` result.
``None`` was being used as the cached result flag and obviously breaks
on a ``None`` returned from the remote target task. This would cause an
infinite hang if user code ever called ``Portal.result()`` *before* the
nursery exit. The simple fix is to use the ``Channel.uid`` as the
initial "no-result-received-yet" flag.
nursery exit. The simple fix is to use the *return message* as the
initial "no-result-received-yet" flag value and, once received, the
return value is read from the message to avoid the cache logic error.
36 changes: 22 additions & 14 deletions tractor/_portal.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ def __init__(self, channel: Channel) -> None:
# when this is set to a tuple returned from ``_submit()`` then
# it is expected that ``result()`` will be awaited at some point
# during the portal's lifetime
self._result: Optional[Any] = channel.uid
self._result_msg: Optional[dict] = None

# set when _submit_for_result is called
self._expect_result: Optional[
Tuple[str, Any, str, Dict[str, Any]]
Expand Down Expand Up @@ -129,12 +130,12 @@ async def _return_once(
resptype: str,
first_msg: dict

) -> Any:
) -> tuple[Any, dict[str, Any]]:
assert resptype == 'asyncfunc' # single response

msg = await recv_chan.receive()
try:
return msg['return']
return msg['return'], msg
except KeyError:
# internal error should never get here
assert msg.get('cid'), "Received internal error at portal?"
Expand All @@ -159,17 +160,21 @@ async def result(self) -> Any:

# expecting a "main" result
assert self._expect_result
if self._result is self.channel.uid:

if self._result_msg is None:
try:
self._result = await self._return_once(*self._expect_result)
result, self._result_msg = await self._return_once(
*self._expect_result)
except RemoteActorError as err:
self._result = err
result = err
else:
result = self._result_msg['return']

# re-raise error on every call
if isinstance(self._result, RemoteActorError):
raise self._result
if isinstance(result, RemoteActorError):
raise result

return self._result
return result

async def _cancel_streams(self):
# terminate all locally running async generator
Expand Down Expand Up @@ -244,22 +249,25 @@ async def run_from_ns(
instance methods in the remote runtime. Currently this should only
be used for `tractor` internals.
"""
return await self._return_once(
value, _ = await self._return_once(
*(await self._submit(namespace_path, function_name, kwargs))
)
return value

async def run(
self,
func: str,
fn_name: Optional[str] = None,
**kwargs
) -> Any:
"""Submit a remote function to be scheduled and run by actor, in
'''
Submit a remote function to be scheduled and run by actor, in
a new task, wrap and return its (stream of) result(s).
This is a blocking call and returns either a value from the
remote rpc task or a local async generator instance.
"""
'''
if isinstance(func, str):
warnings.warn(
"`Portal.run(namespace: str, funcname: str)` is now"
Expand All @@ -285,9 +293,9 @@ async def run(

fn_mod_path, fn_name = func_deats(func)

return await self._return_once(
return (await self._return_once(
*(await self._submit(fn_mod_path, fn_name, kwargs))
)
))[0]

@asynccontextmanager
async def open_stream_from(
Expand Down

0 comments on commit 0e7234a

Please sign in to comment.