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

conn.exportObject() should return deferred object instead of None #20

Open
socketpair opened this issue Nov 20, 2014 · 6 comments
Open

Comments

@socketpair
Copy link

Since this method calls conn.sendMessage() internally, this method may block.

Moreover. Library should expect that socket write function may (!) block! So, whole chain of calls should implement deffered semantics.

@cocagne
Copy link
Owner

cocagne commented Nov 20, 2014

What makes you think the function has the potential to block? All network
communication is implemented in terms of twisted.internet.protocol.Protocol
classes and the framework guarantees that the transport.write() calls will
not block.

Tom

On Thu, Nov 20, 2014 at 1:46 PM, Коренберг Марк [email protected]
wrote:

Since this method calls conn.sendMessage() internally, this method may
block.

Moreover. Library should expect that socket write function may (!) block!
So, whole chain of calls should implement deffered semantics.


Reply to this email directly or view it on GitHub
#20.

@socketpair
Copy link
Author

Yes, you are right.Twsited just buffer that data in its head, and send it when possible. But we should wait when this write aactually complete in order to execute next actions.

I'm coming from Tornado world. Tornado's streams have same .write() (always non-blocking, returns None) and also .flush() which is blocking (i.e. have callback=... argument, and so can be yielded). I'm novice in Twisted framework, but Twisted should have the same. Otherwise we should reinvent wheel by using something like .is_writing() + some wait.

Real case:
I export an object, and try to use it immediatelly in another DBUS-connection in the same application. There is race-condition, since we don't sure that object has been exported completely before using.

@cocagne
Copy link
Owner

cocagne commented Nov 21, 2014

I could see some value to a .flush() method like the one you describe but,
as your example shows, there's a danger to such a method. Suppose you
implemented your example using a .flush() method that works exactly as you
described. You'd still have a race condition. The best a .flush() method
can do is say "the application has handed the data off to the kernel".
Specifically, it does not guarantee that the message is immediately placed
in the DBus daemon's queue, that the daemon will receive the message
immediately, and that the application logic will process it in a timely
manner. Even with a .flush() there's still no guarantee as to the order in
which the messages will be processed. Practically speaking, it would
probably do the right thing 99.99% of the time but it wouldn't be "correct".

It's a nuisance but the only way to really be sure the object has been
completely exported is for the daemon to explicitly tell you it has; and
that probably means polling.

Tom

On Thu, Nov 20, 2014 at 2:59 PM, Коренберг Марк [email protected]
wrote:

Yes, you are right.Twsited just buffer that data in Head, and send it when
possible. But we should wait when this write aactually complete in order to
execute next actions.

I'm coming from Tornado world. Tornado's streams have same .write()
(always non-blocking, returns None) and also .flush() which is blocking
(i.e. have callback=... argument, and so can be yielded). I'm novice in
Twisted framework, but Twisted should have the same. Otherwise we should
reinvent wheel by using something like .is_writing() + some wait.

Real case:
I export an object, and try to use it immediatelly in another
DBUS-connection in the same application. There is race-condition, since we
don't sure that object has been exported completely before using.


Reply to this email directly or view it on GitHub
#20 (comment).

@socketpair
Copy link
Author

I have discovered, that DBUS does not acknowledge that operation. Very sad. BUT. while experimenting I have found, that if I call reactor.stop() next after conn.exportObject(), I will not see corresponding sendto(). Exactly the same with con.disconnect(), where I expect sendto(''), which normally occured when I don't stop reactor.

I did not check, but seems we have race-condtition when someone wants to call some remote function without waiting for the result (expectReply=True) and exit event loop, this will FAIL since corresponding sendto()will not be sent.

@cocagne
Copy link
Owner

cocagne commented Nov 24, 2014

It's definitely a race condition. That workflow isn't really supported by
Twisted. It's more of a "Do this eventually" kind of system than "do this
right now". "reactor.stop()" takes effect immediately so the typical design
involves the application detecting that all outstanding work has completed
and only then calling "reactor.stop()". The in example you provided, the
application is essentially saying "schedule a message for eventual delivery
to the DBus daemon and then immediately shut down".

Instead what you probably want to do is:

  1. Make your remote call with expectReply=False
  2. Register a callback handler with the txdbus client's
    notifyOnDisconnect() method that calls 'reactor.stop()'
  3. Call the client's disconnect() method

That should cleanly shut down the connection to the dbus daemon after
writing all outstanding data to the socket, followed by terminating the
reactor loop. I think anyway, it's not something I've tried but according
to the Twisted APIs, that's how it ought to work.

On Fri, Nov 21, 2014 at 5:10 PM, Коренберг Марк [email protected]
wrote:

I have discovered, that DBUS does not acknowledge that operation. Very
sad. BUT. while experimenting I have found, that if I call reactor.stop()
next after conn.exportObject(), I will not see corresponding sendto().
Exactly the same with con.disconnect(), where I expect sendto(''), which
normally occured when I don't stop reactor.

I did not check, but seems we have race-condtition when someone wants
to call some remote function without waiting for the result (
expectReply=True) and exit event loop, this will FAIL since corresponding
sendto()will not be sent.


Reply to this email directly or view it on GitHub
#20 (comment).

@socketpair
Copy link
Author

Yes, thanks, that works! But still no clean solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants