[core/IPCChannel] : Unregister the handlers in the full chain. #1697
Conversation
…PTestAdministrator'
Handlers are registerd via the client in the 'Accept' method but are 'forgotten' via the 'Cleanup' if already being explicitly unregistered.
…cb61d929c' Including various improvements for some test.
…turnLastPushedMessageInOtherProcess' in test_message_unit'
…annel_dangling_handlers
| @@ -333,6 +333,11 @@ namespace Core { | |||
| ASSERT(index != _handlers.end()); | |||
|
|
|||
| if (index != _handlers.end()) { | |||
| for(auto it = _clients.begin(), tail = _clients.end(); it != tail; it++) { | |||
There was a problem hiding this comment.
This is making the Register/Unregister a-symetrical. The Register records the offered interface, the Unregister should remove it (Symetrical) but now it also starts to Unregister something else? What does this resolve ? I guess the use case might be missing something..
There was a problem hiding this comment.
As communicated, the use case is shown in another branch / open PR. You might have forgotten about it: #1698. It also contains some explanation.
There was a problem hiding this comment.
You might also have a look at the description in https://jira.rdkcentral.com/jira/browse/METROL-1024.
There was a problem hiding this comment.
@msieben Question is if the a-symetrical part (The unregister also now unregisters some Client stuff) should not have been done long before the handler is unregistered and thus the issue should be resolved somewhere else to make the test case succeed. It could be that Clients might still exists at the moment this destructor runs. Than the Clients must be closed before the IPCChannel is destructed.. So could well be an issue but should the unregstering of the client not have happened before the Handlers get unregistered....
There was a problem hiding this comment.
@pwielders I believe your suggested flow is not really apparent from the interface description. Synchronisation has been in place, and, scoping the client has not shown additional improvements. However, reordering the server side flow has. Especially, Cleanup prior Unregister - server side -, apparently is needed to avoid issues, which imho is confusing. Pushed changes.
|
Superseded by #1698 |
Handlers are registerd via the client in the 'Accept' method but are 'forgotten' via the 'Cleanup' if already being explicitly unregistered.