feat: give control on which CORBA connections are served by which dispatchers

[doudou]

This PR allows to use the name_id field of the connection policy to select which dispatcher should be used for this connection, thus allowing to isolate connections (avoiding for instance that a connection to a remote that disappeared to block communication to other hosts)

When doing nothing, a name is generated based on the dataflow object we're connecting with, thus retaining the 1:1 mapping between dispatchers and dataflow interfaces

[pierrewillenbrockdfki]

While i am not a fan of using strings as keys in maps, you mitigated the performance issue around that by introducing long-term references(::Acquire/::Release with reference counters instead of only ::Instance, and a ::Release that magically needs to be called at the end).

In our code base, i found two uses of CorbaDispatcher::Instance in gui/rock__replay and tools/orocos_cpp_base, for changing the Activities scheduler. The lifetime management of that CorbaDispatcher instance is going to happen inside rtt in conjunction with the task, i'd think. If you want to remove CorbaDispatcher::Instance entirely, we'd have to change those.

[doudou]

In our code base, i found two uses of CorbaDispatcher::Instance in gui/rock__replay and tools/orocos_cpp_base, for changing the Activities scheduler. The lifetime management of that CorbaDispatcher instance is going to happen inside rtt in conjunction with the task, i'd think. If you want to remove CorbaDispatcher::Instance entirely, we'd have to change those.

I don't want to remove those. Existing code should work as-is. If you want to start using the feature, and add support for it, then you'd need to setup the default scheduler and priority so that the dynamically created dispatchers are properly

Which is broken, unless I'm mistaken. Essentially, Instance should be an alias to Acquire ... Do you agree ?

[pierrewillenbrockdfki]

Which is broken, unless I'm mistaken. Essentially, Instance should be an alias to Acquire ... Do you agree ?

As far as i can tell, before, the Instance call was creating (semi-)permanent objects that were only supposed to be cleaned up at the end of the of the process/rtt "session"(using ReleaseAll). Since they were found using a DataFlowInterface object, which i assume is created for each task, the CorbaDispatchers could no longer be found once the task and its DataFlowInterface was deleted, where it could as well be Released. So, Instance produced a short term pointer without actually affecting end-of-life of the object, Release deleted a CorbaDispatcher that would no longer be findable afterwards anyways. (I did not check the actual usage of Release.)

After this merge, Instance still does create the CorbaDispatcher on-demand, which potentially would never be cleaned up, providing the short term pointer. It does not matter if it is Acquired before or afterwards, the effect is that whatever is done to the pointer from Instance still affects the right CorbaDispatcher. It is only with Release that things change. An CorbaDispatcher that has been properly Acquired(potentially multiple times) is only deleted when it is Released the same number of times. I assume that the Release is done during task cleanup or similar, so effective behavior does not change.

if we make Instance an alias to Acquire, we would remove Releases ability to cleanup the CorbaDispatcher when Instance is used the old way.

[doudou]

Acquire/Release is done through the lifecycle of the channel element. So, in effect, it's a phoenix singleton - that is, it is created and destroyed as the dispatchers require it.

Which is where I think that this PR breaks stuff, with the old usage workflow... If Instance does not increase the refcount it will be destroyed the next time a channel element releases it - which is not expected in the old workflow. Plus I removed ReleaseAll, which will obviously break code that was using it (which is neither orocos.rb nor orogen actually).

Old code could be doing Instance/Release, but multiple Instance would require a single Release.

I guess the right fix then is to emulate the old Instance behaviour with the new, that is do a single Acquire for any amount of Instance calls, having Release deref that single instance if it is present (resurrecting ReleaseAll), and call Deref the methods with the new behaviour.

What do you think ?

[pierrewillenbrockdfki]

I guess the right fix then is to emulate the old Instance behaviour with the new, that is do a single Acquire for any amount of Instance calls, having Release deref that single instance if it is present (resurrecting ReleaseAll), and call Deref the methods with the new behaviour.

I guess that would work.

I am not sure of the benefit of reaping the Activity when it is no longer needed. When the next user comes around, they again need to spin up the Activity(iirc that creates a thread, mildly expensive, probably not RT) instead of using one that already exists. I think only at the end of the rtt session, one must make sure all Activities are removed so there are no threads left in the process after the main thread ends.

For our use case, i think the task->addConstant stuff from rtt 2.9 could be useful, so all CorbaDispatchers created on behalf of a given task can be (default) configured with priorities/schedulers given through the constants.

[doudou]

For our use case, i think the task->addConstant stuff from rtt 2.9 could be useful, so all CorbaDispatchers created on behalf of a given task can be (default) configured with priorities/schedulers given through the constants.

When you give a name, there is not anymore a 1:1 mapping between task contexts and dispatchers (on purpose).

In practice, the dispatcher will be reaped only if there are no connections using that particular dispatcher in the whole process. I think it's a pretty rare occurence. And it is created at connection time, which is already a (very !) non-RT context.