Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Async callback with custom object causes invalid reference counting handling #153

Closed
haukero opened this issue Dec 8, 2014 · 1 comment
Labels
Bug Confirmed bug

Comments

@haukero
Copy link

haukero commented Dec 8, 2014

Hi All,

the following code makes RPyC 3.3.0 crash on my machine because of too early reference counting decrease.

Please note the following conditions:

  • Synchronous client call that passes a callback function to the server.
  • Asynchronous callback (event) that calls the received callback function.
  • The event passes an object back to the client whose class definition is unknown on the client side.

After debugging the code and tracing the protocol, it seems that the callback function is requested to be deleted from the local objects dictionary on the client side,
before the callback on this function is actually executed. And the callback call is actually delayed because the parameter class definition is unknown on the client side.

Unfortunately, I haven't figured out where to fix the error.

I would assume that the delete request needs to be delayed until the callback finished. Note that the callback is asynchronous.

Thanks for any help,
Rico

Server code:

import rpyc
import os

class URL (str):
    pass

class DownloadService (rpyc.Service):
    def exposed_download_url (self, url, finished):
        a = rpyc.async(finished)
        a(URL(url))

if __name__ == '__main__':
    print os.getpid()
    from rpyc.utils.server import ThreadedServer
    serv = ThreadedServer(DownloadService, port=18812, protocol_config={"allow_all_attrs": True})
    serv.start()

Client code:

----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
import logging
import rpyc

def finished (self, url):
    print "Finished."

if __name__ == '__main__':
    url = "http://www.car-it.com/wp-content/uploads/2014/02/13C1150_092.jpg"
    logging.basicConfig(level=logging.DEBUG)
    log = logging.getLogger(__name__)
    con = rpyc.connect("localhost", port=18812, config={"allow_all_attrs": True, "logger": log})
    con.root.download_url(url, finished)
    # Keep alive.
    while True:
        pass

Client side exception:

pydev debugger: starting (pid: 1988)
DEBUG:__main__:Exception caught
Traceback (most recent call last):
  File "C:\Program Files (x86)\Python27\lib\site-packages\rpyc\core\protocol.py", line 305, in _dispatch_request
    res = self._HANDLERS[handler](self, *args)
  File "C:\Program Files (x86)\Python27\lib\site-packages\rpyc\core\protocol.py", line 535, in _handle_call
    return self._local_objects[oid](*args, **dict(kwargs))
  File "C:\Program Files (x86)\Python27\lib\site-packages\rpyc\lib\colls.py", line 86, in __getitem__
    return self._dict[key][0]
KeyError: 40805104

Protocol trace (seq id, msg id):

Client:                                                                         Server:
-------                                                                           -------

--> REQ                                                                        (0, 1): GetRoot()
(0, 2): (41243912, 'DownloadService', 'pkgs.dlserv')           <-- RESP

--> REQ                                                                        (1, 1): Inspect(41243912)
(1, 2): <List of methods>                                                <-- RESP

--> REQ                                                                        (2, 1): GetAttr(41243912, 'download_url')
(2, 2): (38883696, 'instancemethod', '__builtin__')              <-- RESP

--> REQ                                                                        (3, 1): Call(38883696, url, finished=(40805104, 'function', '__builtin__'))

(0, 1): Call(40805104, url=(41284640, 'URL', '__main__'))    <-- REQ

(3, 2): None                                                                    <-- RESP

(1, 1): Del(40805104)                                                       <-- REQ

--> REQ                                                                         (4, 1): Inspect(41284640)
(4, 2): <Class description>                                               <-- RESP

--> RESP                                                                       (1, 2): None

--> EXCP                                                                       (0, 3): KeyError(40805104)
@coldfix coldfix added the Bug Confirmed bug label Jul 9, 2017
@coldfix
Copy link
Contributor

coldfix commented Dec 22, 2017

Seems to have been solved by 4c8f004 which will be part of the next release. Not sure how reliably.

BTW: client should read:

def finished (url):
    print "Finished."

(without self parameter)

Let me know, if you continue to have problems.

Best, Thomas

@coldfix coldfix closed this as completed Dec 22, 2017
coldfix added a commit that referenced this issue Jun 11, 2018
This release brings a few minor backward incompatibilities, so be sure to read
on before upgrading. However, fear not: the ones that are most likely relevant
to you have a relatively simple migration path.

Backward Incompatibilities
^^^^^^^^^^^^^^^^^^^^^^^^^^

* ``classic.teleport_function`` now executes the function in the connection's
  namespace by default. To get the old behaviour, use
  ``teleport_function(conn, func, conn.modules[func.__module__].__dict__)``
  instead.

* Changed signature of ``Service.on_connect`` and ``on_disconnect``, adding
  the connection as argument.

* Changed signature of ``Service.__init__``, removing the connection argument

* no longer store connection as ``self._conn``. (allows services that serve
  multiple clients using the same service object, see `#198`_).

* ``SlaveService`` is now split into two asymetric classes: ``SlaveService``
  and ``MasterService``. The slave exposes functionality to the master but can
  not anymore access remote objects on the master (`#232`_, `#248`_).
  If you were previously using ``SlaveService``, you may experience problems
  when feeding the slave with netrefs to objects on the master. In this case, do
  any of the following:

  * use ``ClassicService`` (acts exactly like the old ``SlaveService``)
  * use ``SlaveService`` with a ``config`` that allows attribute access etc
  * use ``rpyc.utils.deliver`` to feed copies rather than netrefs to
    the slave

* ``RegistryServer.on_service_removed`` is once again called whenever a service
  instance is removed, making it symmetric to ``on_service_added`` (`#238`_)
  This reverts PR `#173`_ on issue `#172`_.

* Removed module ``rpyc.experimental.splitbrain``. It's too confusing and
  undocumented for me and I won't be developing it, so better remove it
  altogether. (It's still available in the ``splitbrain`` branch)

* Removed module ``rpyc.experimental.retunnel``. Seemingly unused anywhere, no
  documentation, no clue what this is about.

* ``bin/rpyc_classic.py`` will bind to ``127.0.0.1`` instead of ``0.0.0.0`` by
  default

* ``SlaveService`` no longer serves exposed attributes (i.e., it now uses
  ``allow_exposed_attrs=False``)

* Exposed attributes no longer hide plain attributes if one otherwise has the
  required permissions to access the plain attribute. (`#165`_)

.. _#165: #165
.. _#172: #172
.. _#173: #173
.. _#198: #198
.. _#232: #232
.. _#238: #238
.. _#248: #248

What else is new
^^^^^^^^^^^^^^^^

* teleported functions will now be defined by default in the globals dict

* Can now explicitly specify globals for teleported functions

* Can now use streams as context manager

* keep a hard reference to connection in netrefs, may fix some ``EOFError``
  issues, in particular on Jython related (`#237`_)

* handle synchronous and asynchronous requests uniformly

* fix deadlock with connections talking to each other multithreadedly (`#270`_)

* handle timeouts cumulatively

* fix possible performance bug in ``Win32PipeStream.poll`` (oversleeping)

* use readthedocs theme for documentation (`#269`_)

* actually time out sync requests (`#264`_)

* clarify documentation concerning exceptions in ``Connection.ping`` (`#265`_)

* fix ``__hash__`` for netrefs (`#267`_, `#268`_)

* rename ``async`` module to ``async_`` for py37 compatibility (`#253`_)

* fix ``deliver()`` from IronPython to CPython2 (`#251`_)

* fix brine string handling in py2 IronPython (`#251`_)

* add gevent_ Server. For now, this requires using ``gevent.monkey.patch_all()``
  before importing for rpyc. Client connections can already be made without
  further changes to rpyc, just using gevent's monkey patching. (`#146`_)

* add function ``rpyc.lib.spawn`` to spawn daemon threads

* fix several bugs in ``bin/rpycd.py`` that crashed this script on startup
  (`#231`_)

* fix problem with MongoDB, or more generally any remote objects that have a
  *catch-all* ``__getattr__`` (`#165`_)

* fix bug when copying remote numpy arrays (`#236`_)

* added ``rpyc.utils.helpers.classpartial`` to bind arguments to services (`#244`_)

* can now pass services optionally as instance or class (could only pass as
  class, `#244`_)

* The service is now charged with setting up the connection, doing so in
  ``Service._connect``. This allows using custom protocols by e.g. subclassing
  ``Connection``.  More discussions and related features in `#239`_-`#247`_.

* service can now easily override protocol handlers, by updating
  ``conn._HANDLERS`` in ``_connect`` or ``on_connect``. For example:
  ``conn._HANDLERS[HANDLE_GETATTR] = self._handle_getattr``.

* most protocol handlers (``Connection._handle_XXX``) now directly get the
  object rather than its ID as first argument. This makes overriding
  individual handlers feel much more high-level. And by the way it turns out
  that this fixes two long-standing issues (`#137`_, `#153`_)

* fix bug with proxying context managers (`#228`_)

* expose server classes from ``rpyc`` top level module

* fix logger issue on jython

.. _#137: #137
.. _#146: #146
.. _#153: #153
.. _#165: #165
.. _#228: #228
.. _#231: #231
.. _#236: #236
.. _#237: #237
.. _#239: #239
.. _#244: #244
.. _#247: #247
.. _#251: #251
.. _#253: #253
.. _#264: #264
.. _#265: #265
.. _#267: #267
.. _#268: #268
.. _#269: #269
.. _#270: #270

.. _gevent: http://www.gevent.org/
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Confirmed bug
Projects
None yet
Development

No branches or pull requests

2 participants