mbedtls icon indicating copy to clipboard operation
mbedtls copied to clipboard

Test step functions may not function as expected in a multithreaded environment

Open paul-elliott-arm opened this issue 1 year ago • 2 comments

Suggested enhancement

Although they are thread safe, calling mbedtls_test_increment_step() and particularly mbedtls_test_set_step() from within mutliple threads at the same time may not have the desired effects, as we cannot control the order of execution.

Its not going to be entirely simple to fix this - we could possibly do it by making all the test data thread specific data, although how we do this in a cross platform manner I am not entirely sure at the time of writing. Unfortunately we also have test functions that do some tests, spawn threads to do some other tests, and then wait for those tests to complete before doing final tests, this won't entirely work well with the above situation, especially as there is now the chance that a test failure during the threaded portion of the test can get ignored (the main thread data is currently the only one that will be displayed)

Its possible that drivers will cause even more problems here in future as well.

For now we have fixed this by adding comments to warn the user, but it would be nice to fix up the test data properly, although as I say how, and the size of this work is yet to be ascertained.

paul-elliott-arm avatar Feb 09 '24 14:02 paul-elliott-arm

Out of curiosity, why is reporting the test step more difficult than reporting the file/line location?

gilles-peskine-arm avatar Feb 09 '24 15:02 gilles-peskine-arm

Reporting the line / location is guarded by the test result state - we take the lock, then set the test name, line and file (and the two extra lines if necessary), and then unlock. Should a second thread come along at this point, they would bounce off as the test had already failed.

When setting the step (to an arbitary value) there is no such built in safeguard, one simply sets the step. If two threads are both attempting to set the step, then the order in which they set the step (one obviously overwriting the other) is completely undetermined. Even if we constrained the step value to a single test, and that test used threads we would hit this, so I'm really not entirely sure, as I said how best to fix it.

In the case of increment step, if its basically being used as a counter, then it will function as that, but there is still uncertainty about the order in which increments happen, however if that is not something you are bothered about then this is still useful in a multi-thread environment.

paul-elliott-arm avatar Feb 15 '24 08:02 paul-elliott-arm