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

Fix broken addListener() & delListener() handling. #1415

Merged
merged 1 commit into from
Sep 19, 2018

Conversation

varumugam123
Copy link
Contributor

@varumugam123 varumugam123 commented Aug 23, 2018

Now every time client makes a call to delListener(), rtFunctionRef() gets new ID and server doesn't identify it when it matches it with the stored handlers.

This change will ensure at client rtFuncionRef stores the id after creation and refers that hence forth instead of creating new one everytime. And at the server side, the received
id (i.e. 'func://') is used to calculate a hash which is then used for add / del listeners

Now when client makes a call to delListener() everytime rtFunctionRef() is created with new
Id and server doesn't remove any handler. This change will ensure everytime rtFuncionRef is
created for rtFunctionCallback it gets the same Id. Now at the server side, the received
name (i.e. 'func://<uid>') is used to calculate a hash which is then used for add / del listners
@varumugam123
Copy link
Contributor Author

Sample Server & Client to verify this:

rtSampleClient.txt
rtSampleServer.txt

@varumugam123
Copy link
Contributor Author

Output of the attached sample server is :

madan@madan-VirtualBox:~/pxscene/pxCore/remote$
 ./rtSampleServer
rt: WARN rtSampleServer.cpp:29 -- Thread-25823: 0xf098a0 (func://9edc2334-45c2-4c28-bd64-c6d46c1ae3fb) is added for "onTestEvent" event
rt: WARN rtSampleServer.cpp:29 -- Thread-25823: 0xf09a00 (func://db67db85-3ef3-4a87-b1ca-a209412aca9c) is added for "onTestEvent" event
rt: WARN rtSampleServer.cpp:29 -- Thread-25823: 0xf09960 (func://e168cfcf-4919-408c-853e-5085c8c537e7) is added for "onTestEvent" event
rt: WARN rtSampleServer.cpp:29 -- Thread-25823: 0xf09ad0 (func://a5b06bf1-a8b6-4215-92a3-ebcdcae8447e) is added for "onTestEvent" event
rt: WARN rtSampleServer.cpp:29 -- Thread-25823: 0xf09d60 (func://b54909db-5acc-4a0b-ac70-48e4760d665c) is added for "onTestEvent" event
rt: WARN rtSampleServer.cpp:42 -- Thread-25823: 0xefac30 is not removed from "onTestEvent" event
rt: WARN rtSampleServer.cpp:40 -- Thread-25823: 0xefac30 (func://9edc2334-45c2-4c28-bd64-c6d46c1ae3fb) is removed from "onTestEvent" event
rt: WARN rtSampleServer.cpp:40 -- Thread-25823: 0xefac00 (func://db67db85-3ef3-4a87-b1ca-a209412aca9c) is removed from "onTestEvent" event
rt: WARN rtSampleServer.cpp:40 -- Thread-25823: 0xf09a00 (func://e168cfcf-4919-408c-853e-5085c8c537e7) is removed from "onTestEvent" event
rt: WARN rtSampleServer.cpp:40 -- Thread-25823: 0xefab70 (func://a5b06bf1-a8b6-4215-92a3-ebcdcae8447e) is removed from "onTestEvent" event
rt: WARN rtSampleServer.cpp:40 -- Thread-25823: 0xefab70 (func://b54909db-5acc-4a0b-ac70-48e4760d665c) is removed from "onTestEvent" event

Thanks @madanagopalt for helping with testing this change on latest code.

@varumugam123
Copy link
Contributor Author

#1111 seems to be breaking the "addListener()" functionality on native rtRemote component. The above PR initializes m_Hash of all the rtRemoteFunction objects to 0, which causes addListener() to skip adding the handlers to the list.

The proposed solution initializes m_Hash with the hash of its name (a unique id "func://xyz"), this way ensures both addListener() & delListener() works fine.

@varumugam123 varumugam123 changed the title Fix broken delListener() handling. Fix broken addListener() & delListener() handling. Aug 27, 2018
@gladish gladish merged commit fb71123 into pxscene:master Sep 19, 2018
@dwrobel
Copy link
Contributor

dwrobel commented Sep 26, 2018

Shouldn't that also address/fix #1113 (original regression caused by #1041 and duplicate listeners behaviour)?

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

Successfully merging this pull request may close these issues.

3 participants