application can't create publisher repeatedly with previous one already destroyed

[qclzdh]

Required information

Operating system:
Ubuntu 20.04 LTS

Compiler version:
GCC 9.3.0

Observed result or behaviour:
test application reported some error log:
[ Error ]: Request publisher received no valid publisher port from RouDi
[Warning]: Service '10:Radar-test14:FrontLeft-test6:Object1:01:01:01:01:01:0' already in use by another process.
[Warning]: ICEORYX error! POSH__RUNTIME_PUBLISHER_PORT_NOT_UNIQUE
Roudi log:
2021-10-08 15:27:33.443 [ Debug ]: Registered new application iox-cpp-publisher-helloworld
2021-10-08 15:27:33.444 [ Debug ]: Created new ApplicationPort for application iox-cpp-publisher-helloworld
2021-10-08 15:27:33.445 [ Debug ]: Created new PublisherPort for application iox-cpp-publisher-helloworld
2021-10-08 15:27:33.446 [ Debug ]: Created new PublisherPort for application iox-cpp-publisher-helloworld
2021-10-08 15:27:33.446 [Warning]: Process 'iox-cpp-publisher-helloworld' violates the communication policy by requesting a PublisherPort which is already used by 'iox-cpp-publisher-helloworld' with service '10:Radar-test14:FrontLeft-test6:Object1:01:01:01:01:01:0'.
2021-10-08 15:27:33.446 [Warning]: ICEORYX error! POSH__PORT_MANAGER_PUBLISHERPORT_NOT_UNIQUE
2021-10-08 15:27:33.446 [ Error ]: Could not create PublisherPort for application iox-cpp-publisher-helloworld
2021-10-08 15:27:33.541 [ Debug ]: Destroyed publisher port

Expected result or behaviour:
I expect the same publisher should be created successfully while previous one already destroyed.

Conditions where it occurred / Performed steps:

1. re-build whole iceoryx with "ONE_TO_MANY_ONLY=ON"
2. add following code in iceoryx_examples/icehello/iox_publisher_helloworld.cpp
   iox::capro::ServiceDescription serviceDescription{"Radar-test", "FrontLeft-test", "Object"}; std::shared_ptr<iox::popo::Publisher<RadarObject>> p_publisher = std::make_shared<iox::popo::Publisher<RadarObject>>(serviceDescription); p_publisher->stopOffer(); p_publisher.reset(); p_publisher = std::make_shared<iox::popo::Publisher<RadarObject>>(serviceDescription);
3. build and run, roudi and iox-cpp-publisher-helloworld

[elBoberido]

@qclzdh currently we do not communicate the destruction of a port to RouDi to release the resources. The cleanup is done when the application stops and unregisters at RouDi or when an application crashes and RouDi does the cleanup. Up until now we did not have such a use case like you described because the publisher always lived as long as the application.

What is the use case for the scenario you described?

[elBoberido]

@qclzdh forget what I wrote about the release of the resource. It is released, but with a delay. In order to not block the destructor of the publisher only a flag is set and in the so called RouDi discovery loop the port is reclaimed by RouDi. So when you wait some time before the publisher is recreated it works.

iox::capro::ServiceDescription serviceDescription{"Radar-test", "FrontLeft-test", "Object"};
auto p_publisher = std::make_shared<iox::popo::Publisher<RadarObject>>(serviceDescription);
p_publisher->stopOffer();
p_publisher.reset();
std::this_thread::sleep_for(std::chrono::milliseconds(2 * iox::roudi::DISCOVERY_INTERVAL.toMilliseconds()));
p_publisher = std::make_shared<iox::popo::Publisher<RadarObject>>(serviceDescription);

We could optimize this at the cost of slightly increasing the latency of creating new ports by checking the destruction flag for all the existing publisher ports and let RouDi reclaim a port which is marked for destruction before a new one is created

@budrus @elfenpiff @MatthiasKillat what do you think?

[elfenpiff]

@elBoberido I completely agree. As a developer I would expect that the publisher is fully destroyed and all the resources are released when it is out of scope or the destructor should block the process until this is ensured.
The best way to mimic this behavior would be that RouDi checks if there are any ports marked for destruction before it creates a new one. The fastest way of implementing this would not to reclaim it, it would be delete ports marked for destruction first and then create a new one and since we are not on the hot path I would prefer this approach.

[elBoberido]

@elfenpiff right. With reclaim I mean exactly what you wrote

[budrus]

@elfenpiff @elBoberido This is a consequence of the delayed destruction in the discovery loop. I forgot the reasons why we did it like this and do not block the d'tor and wait for RouDi to acknowledge the removal of the old port. Maybe to avoid the races with the discovery loop? I think we should fix this for the 2.0 release. Either like you propose by having a discovery (light) when creating a new port or by refactoring the cleanup

[budrus]

@qclzdh FYI, will be fixed with the upcoming 2.0 release

[qclzdh]

Thanks~