bug - agent src/agent/dtls_mbedtls.{ch}pp

[DuaneEllis-TI]

What should happen is - here at:

https://github.com/openthread/borderrouter/blob/master/src/agent/dtls_mbedtls.hpp#L224

In the MbedtlsServer() Constructor, the member variable: 'mSocket' should be initialized to (-1)

Instead, it takes on a RANDOM value based upon what ever the run time environment leaves there.

What happens later, during the call to void MbedtlsServer::UpdateFdSet()

Code: https://github.com/openthread/borderrouter/blob/master/src/agent/dtls_mbedtls.cpp#L411

The RANDOM value is used as a BIT index into the various FDSETS - which then has a 50/50 chance of flipping a bit somewhere on the run-time stack.

I say 50/50 - because the bit may already be 1, and the operation here is to SET the bit, the best part of this: Is if you change the code to debug this the problem has a chance of moving, or going away.

What happens sometimes (especially in the commissioner test application) the first call to the void MbedtlsServer::UpdateFdSet() the socket is not yet open, very soon after - and and this depends on other timing items, mSocket is initialized with some valid socket number.

The problem occurs here:

https://github.com/openthread/borderrouter/blob/master/src/agent/dtls_mbedtls.cpp#L458

Note: Appropriate "valid-fd-test" needs to be inserted at the FD_SET operation

FYI @pkanek & @srickardti

[bukepo]

Thanks for reporting this issue. Does this problem only exist in the functional test commissioner? I noticed that in tests/meshcop/commissioner.cpp, there missed a call to DtlsServer::Start(). Besides, we could probably refine implementation of DtlsServer so that when Start() is not called, error should occur. Are you going to submit a PR for this issue?

[DuaneEllis-TI]

I found this in the commissioner, I do see the missing Start() - perhaps that limits this to the test commissioner app only.

[bukepo]

@DuaneEllis-TI are you planning to submit a pull request to solve this issue?

[DuaneEllis-TI]

@bukepo - I am going to close this issue because this PULL request resolved it.

#90