openthread / ot-commissioner

OpenThread Commissioner, a Thread commissioner for joining new Thread devices and managing Thread networks.
https://openthread.io/
BSD 3-Clause "New" or "Revised" License
52 stars 35 forks source link

[dtls] handle events when disconnected #115

Closed wgtdkp closed 4 years ago

wgtdkp commented 4 years ago

This PR fixes DTLS events handling when the session is disconnected. Previous implementation has the assumption that the underlying socket will be closed when the session is disconnected. But with PR https://github.com/openthread/ot-commissioner/pull/110, the socket now may be open in this case.

This PR ignores the incoming data when the DTLS session is disconnected.

codecov-commenter commented 4 years ago

Codecov Report

Merging #115 into master will increase coverage by 23.44%. The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           master     #115       +/-   ##
===========================================
+ Coverage   38.52%   61.96%   +23.44%     
===========================================
  Files          58       58               
  Lines        5368     5369        +1     
===========================================
+ Hits         2068     3327     +1259     
+ Misses       3300     2042     -1258     
Impacted Files Coverage Δ
src/library/dtls.cpp 79.03% <100.00%> (+8.00%) :arrow_up:
src/library/coap.cpp 72.13% <0.00%> (+2.55%) :arrow_up:
src/library/timer.hpp 100.00% <0.00%> (+3.57%) :arrow_up:
src/common/error.cpp 23.91% <0.00%> (+4.34%) :arrow_up:
src/app/json.cpp 49.78% <0.00%> (+5.19%) :arrow_up:
src/library/coap.hpp 94.18% <0.00%> (+5.81%) :arrow_up:
src/library/socket.cpp 91.08% <0.00%> (+5.94%) :arrow_up:
src/library/commissioner_impl.hpp 100.00% <0.00%> (+7.69%) :arrow_up:
src/library/dtls.hpp 100.00% <0.00%> (+10.00%) :arrow_up:
... and 21 more
simonlingoogle commented 4 years ago

Previous implementation has the assumption that the underlying socket will be closed when the session is disconnected. But with PR #110, the socket now may be open in this case.

Any background on why the underlying socket should not be closed when DTLS session is disconnected?

wgtdkp commented 4 years ago

Any background on why the underlying socket should not be closed when DTLS session is disconnected?

The Socket class is now applying RAII to manage its fd which means the socket is closed only when the socket object is destructed. We need to add Socket::Reset back if we allow closing a Socket before destruction. Thoughts? @simonlingoogle