Using alternative thread for callbacks

[neilcsmith_net]

Hi,

 

In short - would it be possible in future to extend the (Java)  IPConnection API to allow a different external thread to call in and drain the callback queue?

 

In full - I'm currently working hard on v2 of Praxis LIVE. One major feature coming is that (almost) all components will be defined and compiled at runtime using a new custom code API - a big step towards my original vision of a visual patcher environment where you can fork and alter / extend any component as a patch runs.  It should also make it easier for anyone to provide bindings for additional TinkerForge components that I don't have access to test.

 

Now, the problem is that each patch root in Praxis LIVE is single threaded, and currently the TinkerForge bindings have to handle callbacks by re-queueing and invoking code on the right thread.  Opening this up as-is feels far too complex and error prone.  For now, I may try hacking an in-package extension of IPConnection that overrides callDeviceListener(), but it would be nice to have a proper and more efficient API to handle this sort of use case.

 

Incidentally, another major feature in v2 will be distributed hubs, allowing patches to run across multiple machines - I'm hoping the RED brick might be capable of running a Praxis slave! 

 

Best wishes, Neil

[borg]

This currently not planned, but it wouldn't be hugely complicated to implement. Perhaps we can implement it as an "inofficial" feature with the next Java Bindings release.

[photron]

We could probably do something similar to the manual callback dispatch in PHP: IPConnection::dispatchCallbacks()

 

This could be done by adding the following methods to the IPConnection class:

 

void setAutomaticCallbackDispatch(boolean enabled);
boolean getAutomaticCallbackDispatch();
void dispatchCallbacks(int duration);

[neilcsmith_net]

Thanks for the responses.

 

We could probably do something similar to the manual callback dispatch in PHP:

 

That would possibly meet my needs, though I was really looking to do this without the external thread needing to poll.  Rather than setAutomaticCallbackDispatch(false), what about a setCallbackDispatcher(CallbackDispatcher ..) method that can be set on IPConnection, with an interface called every time a callback is added to the queue?

 

interface CallbackDispatcher {
  public void callbackReceived();
}

 

Then you could do things like

 

SwingCallbackDispatcher implements CallbackDispatcher {
  public void callbackReceived() {
    EventQueue.invokeLater(new Runnable() {
      dispatchCallbacks();
    }
  }
}

 

And get all callbacks on the Swing event queue?  Likewise for JavaFX.  It could simplify having to duplicate this in the body of every callback if you need to update a UI.

 

You could possibly even simplify the current CallbackThread code by replacing with a default CallbackDispatcher that has a single threaded executor - should handle Exceptions by itself?

 

Just a thought ..

 

Thanks, Neil

[photron]

It's not that simple.

 

The internals of the IPConnection have to work in certain ways. We cannot just replace the whole CallbackThread with a user controlled construct, because the CallbackThread does more than just dispatching callback, it's responsible for some internal book keeping. The CallbackThread has to stay.

 

But we could add an "inofficial" CallbackDispatcher interface for the CallbackThread. This lets you inject code between the CallbackThread  and the actual dispatch of the callback:

 

ipcon.setCallbackDispatcher(new IPConnection.CallbackDispatcher() {
public void callbackReceived(IPConnection.Callback callback) {
	callback.dispatch();
}
});

 

The callbacks are still dispatched by the CallbackThread by default as before. But if you set a CallbackDispatcher then the CallbackThread will pass callback objects to it and it has to call their dispatch method to dispatch them. Here you can pass them to your Swing event queue and make the event loop call the dispatch function.

 

Be aware that you need to ensure that all callbacks delivered to the dispatcher are dispatched at some point, otherwise you can block a disconnect call.

 

Attached is a first prototype for this.

tinkerforge_java_bindings_2_1_2_b3f88827594183802a72f623c4dfcdeb7e3c33f7+1.zip

[neilcsmith_net]

It's not that simple.

 

I was aware of that - I said "possibly". 

 

Attached is a first prototype for this.

 

Looks interesting.  Haven't tried yet (will test), but initial question would be - doing it this way does the CallbackThread have to wait on the callback dispatch - presume this may be to do with the dispatchMeta() reconnection management?  In fact, having the reconnection code running on an event thread might be a bad idea.  I wonder if this may be too complicated and invasive an approach?

 

What about the listeners themselves?  Can you foresee an issue with them being called asynchronously?  The other option (as per my initial post) that would work for me would be to be able to override callDeviceListener(..)  Could all the abstract call.. methods in IPConnectionBase be made protected so that we can override them?  Can you envisage any issue with doing this?

 

eg.

 

protected void callDeviceListener(final Device device,
            final byte functionID,
            final byte[] data) {
  EventQueue.invokeLater(new Runnable() {
    IPConnection.super.callDeviceListener(device, functionID, data);
  }
}

 

Here you can pass them to your Swing event queue and make the event loop call the dispatch function.

 

JFTR, I'm not looking specifically to invoke them on the Swing thread - each Praxis root has its own event thread and similar invokeLater(..) method.

[photron]

In the prototype the CallbackThread doesn't wait for the dispatch of the callbacks. In contrast to the released Java bindings that do synchronous dispatch. Asynchronous dispatch should be okay, but it could change the external behavior of the bindings, which may or may not be a problem.

 

Currently I cannot think of any critical problem with asynchronous dispatch except one. The current reconnect logic in the CallbackThread requires the callDisconnectedListeners() method to dispatch the callback synchronously. But this could be fixed.

 

With the prototype the reconnect logic runs in your event logic, instead of the CallbackThread. In a final version this would have to be changed.

 

You might be right, this gets complex fast. So the much simpler approach could be to make the call*Listener() methods protected so you can override them as you suggested. Then I could also make the API contract for this methods says that you have to do synchronous dispatch. Maybe something like this naive approach (creating semaphores might be expensive):

 

protected void callDeviceListener(final Device device, final byte functionID, final byte[] data) {
  final Semaphore semaphore = new Semaphore(0);

  EventQueue.invokeLater(new Runnable() {
    IPConnection.callDeviceListener(device, functionID, data);
    semaphore.release();
  }

  semaphore.acquire();
}

 

Then you have to do all the hard work. That's okay for me

 

As a side note: The IPConnection class is split into IPConnectionBase and IPConnection because the Java bindings are used with some modifications as MATLAB/Octave bindings. This is why the call*Listener() methods are abstract in IPConnectionBase and the actual implementation is in different IPConnection classes. This allows the MATLAB/Octave bindings to implement them in the proper way for MATLAB/Octave.

 

Attached is a second simpler prototype for your override suggestion.

 

Edit: I cannot see any problem with this override approach. The order of execution in the bindings doesn't change. Only the callback dispatch might be slightly delayed because of the round trip through your event loop, but that should not be a problem at all.

tinkerforge_java_bindings_2_1_2_b3f88827594183802a72f623c4dfcdeb7e3c33f7+2_simpler.zip

[neilcsmith_net]

Great!  Thanks for this. 

 

In the prototype the CallbackThread doesn't wait for the dispatch of the callbacks.

 

If you mean your previous prototype, I thought that was the point of the Semaphore and while loop?  I can understand the problem with trying to execute the dispatchMeta reconnection stuff asynchronously.

 

Currently I cannot think of any critical problem with asynchronous dispatch except one. The current reconnect logic in the CallbackThread requires the callDisconnectedListeners() method to dispatch the callback synchronously. But this could be fixed.

 

Why is this required though?  Surely this is just a notification that a disconnect has happened.  I guess it may limit some things you can do in the listener.

 

Then I could also make the API contract for this methods says that you have to do synchronous dispatch.

 

I was hoping to get away from synchronous dispatch if possible, though can handle that if it's the only way.  I had looked a bit at this approach before my first message, and couldn't see any obvious issues with asynchronous dispatch - the data passed into the call.. methods doesn't seem to be reused anywhere (eg. the data array is created each time as far as I can tell).

 

Then you have to do all the hard work. That's okay for me

 

And me! This should save me the hard work of trying to keep a fork of the bindings up-to-date anyway.

 

Thanks again, Neil

[photron]

In the prototype the CallbackThread doesn't wait for the dispatch of the callbacks.

 

If you mean your previous prototype, I thought that was the point of the Semaphore and while loop?  I can understand the problem with trying to execute the dispatchMeta reconnection stuff asynchronously.

 

My first prototype for this was just a quick and dirty one. The CallbackThread did not wait for every dispatch directly, so the dispatch was asynchronous compared to the release version of the bindings. But it did wait at the end for outstanding dispatches with this crude semaphore and while loop pattern, to ensure that all callbacks are dispatched before the IPConnection gets disconnected and the CallbackThread ends. As I said: quick and dirty

 

Currently I cannot think of any critical problem with asynchronous dispatch except one. The current reconnect logic in the CallbackThread requires the callDisconnectedListeners() method to dispatch the callback synchronously. But this could be fixed.

 

Why is this required though?  Surely this is just a notification that a disconnect has happened.  I guess it may limit some things you can do in the listener.

 

It is not strictly required. It's only required if you want to maintain the way it currently works. Currently all bindings guarantee, that the disconnected callback gets dispatched after the socket was properly closed and before auto-reconnect (if enabled) starts. This means in the user-function bound to the disconnect callback you'll see the IPConnection in disconnected state. You can do a manual reconnect or enable/disable auto-reconnect their and it'll work as expected. Even though this is not a documented feature, someone might rely on it, so I try to avoid breaking this.

 

The disconnected callback is the only callback that has to be dispatched synchronously if you want to maintain this behavior. Which means if you don't need this you can dispatch the disconnected callback asynchronously. All other callbacks can be dispatched asynchronously without problems, as they aren't involved in such special behaviors.

 

Then I could also make the API contract for this methods says that you have to do synchronous dispatch.

 

I was hoping to get away from synchronous dispatch if possible, though can handle that if it's the only way.  I had looked a bit at this approach before my first message, and couldn't see any obvious issues with asynchronous dispatch - the data passed into the call.. methods doesn't seem to be reused anywhere (eg. the data array is created each time as far as I can tell).

 

You only need to dispatch all callbacks synchronous if you want to keep the behavior of the bindings completely unchanged. The only callback for which synchronous dispatch really matters is the disconnected callback. If you don't rely on the described behavior then you can dispatch all callback asynchronous. I don't see any problem with this approach, except from changes in the exact timing of things, which shouldn't be a problem normally.

 

To sum this up: You can dispatch all callbacks asynchronously. There is no problem with this. Just keep in mind that this can change the exact circumstances under which the DisconnectedListeners are called.

 

I changed the comment in the code to reflect this better:

 

// to preserve the exact current behavior of the IPConnection all of the
// following abstract methods have to operate synchronous. all listeners
// have to be called before the methods return. this is especially important
// for the DisconnectedListener, because the callback thread expects that
// the user got informed about the disconnect event before it starts the
// auto-reconnect logic. this is because the user shall get a chance to act
// upon the disconnect event before the auto-reconnect logic starts.
//
// synchronous dispatch of the callback is only important if the current
// behavior should be preserved exactly. all callbacks can also be dispatched
// asynchronously. the only negative effect of asynchronous dispatch is
// that the current order of operations around the disconnected callback
// cannot be guaranteed anymore.

[neilcsmith_net]

To sum this up: You can dispatch all callbacks asynchronously. There is no problem with this. Just keep in mind that this can change the exact circumstances under which the DisconnectedListeners are called.

 

Fantastic!  Just what's required.  I'll do some testing over the next week or so, and hopefully not uncover any issues.

 

Thanks, Neil

[photron]

Did you test this yet? Any feedback on this?

[neilcsmith_net]

Did you test this yet?

 

Not enough!   Sorry, the last few weeks have been hectic (I also work on Drupal websites, and you might have seen the rather major security flaw that hit last month!).

 

So, now alpha 3 of Praxis LIVE v2 is out, I'll be working over the next few weeks on things that will properly test this - enabling live coding of TinkerForge bindings is scheduled for alpha 4.

 

Incidentally, alpha 3 finally made distributed hubs a reality - I'm hoping this will (amongst other things) be good for working with the RED bricks.

 

 

[neilcsmith_net]

OK, getting there! 

 

So, the initial switch to working with this seems to be working fine - no issues so far.  I'm now working on integrating the live code support (as per my first post), which will really test this.

 

Two related (on my end) things -

 

* As the live code works by creating a delegate on each compile, the Device is injected as a field.  To counter the possibility of hanging listeners causing the old delegate not to be GC'd, I'm thinking of creating a new Device each time to inject.  Is creating a new Device the only way to disconnect a Device from IPConnection, and more importantly force its listeners to be disconnected?  This could potentially be creating 10's or 100's of devices bound to the same UID as the user iterates code.  Any issues with doing this?

 

* Is there a map of device identifier to Java class anywhere?  Slightly hoping not as I wrote one yesterday and it was tedious!    It could be a good addition to the bindings?  (I'm creating the Device subclasses using reflection)

 

Thanks for all your help and support, Neil