ObservationServiceImpl.cancelObservation leads to duplicated calls to cancelObservation

[cyril2maq]

Version(s)

fa38111

Which components

leshan server core

Tested With

No response

What happened

It happens during the Cancel Observation use case.

https://github.com/eclipse/leshan/blob/fa38111cf1d4787ead04bd04002c2df7cccc8dc1/leshan-server-core/src/main/java/org/eclipse/leshan/server/observation/ObservationServiceImpl.java#L124-L141

The way LwM2mServerEndpoint are built, they share the RegistrationStore and ObservationListener with ObservationServiceImpl (it is built in LeshanServer).

This method starts by removing observation from the store; then calling each endpoint to do the same with their own 'cancelObservation' method. It leads to this call :

https://github.com/eclipse/leshan/blob/fa38111cf1d4787ead04bd04002c2df7cccc8dc1/leshan-server-cf/src/main/java/org/eclipse/leshan/server/californium/observation/LwM2mObservationStore.java#L87-L91

But as the observation has already been deleted, it leads to removedObservation = null. And listeners are called with this 'null' observation.

The same listeners will then be called again in ObservationServiceImpl (line 139, this time with a non-null observation).

How to reproduce

Create a Leshan server with Coap and Coaps endpoints; setup with an ObservationListener :
leshanServer.getObservationService().addListener(observationListener);

With leshan-client, create an observation, then cancel it : your ObservationListener will receive 2 times the cancelled(Observation observation) callback with 'null' observation (1 for Coap endpoint, 1 for Coaps endpoint); then a last callback with the actual cancelled observation (from ObservationServiceImpl).

Relevant Output

No response

[sbernard31]

Great more issue to fix ! 😋
@cyril2maq Thx again 🙏 that really helps !

I will work on it now.

[sbernard31]

Not really done on purpose but I think that #1388 fix this too 🤔

About adding test for multiple call. I think I will maybe do it after #1381 and I will probably give a try using mockito

[cyril2maq]

I agree, in #1388 , with the GET then REMOVE mechanism you did add a null check after the GET which will skip the 'listener.cancelled' call.

https://github.com/eclipse/leshan/blob/c93c16de4761211494f0af4b0ada2e8585584586/leshan-server-cf/src/main/java/org/eclipse/leshan/server/californium/observation/LwM2mObservationStore.java#L87-L100

So no more duplicated calls to the cancelled callback !

[sbernard31]

About adding test for multiple call. I think I will maybe do it after #1381 and I will probably give a try using mockito

I created an issue #1390.

[sbernard31]

I guess we can close this one as #1388 is now integrated in master.

Feel free to reopen, if I'm wrong.

Do not hesitate to provide more feedback or bugs reports ! We really appreciate this !