[Ovmsdev] Metrics & Notifications: thread safety

[Jakob Löw]

Hey,

OvmsMetricBool, -Int and -Float don't seem to use a mutex, but afaik
there is no garantuee for them to be atomic. A simple fix would be to
make m_value a std::atomic<float> etc. or use a mutex just as with
OvmsMetricString.


Furthermore there might be a small race condition when two tasks modify
a metric. Assuming the following base situation:

OvmsMetricString dummy("dummy");
dummy.SetValue("a");

// some other code...

dummy.SetValue("a");
// now between ovms_metrics.cpp line 793 and 794 the thread is interrupted and this happens:

dummy.SetValue("b");
// the value is modified, all change listener are called but none resets the modified flag.

// now execution continues with line 794 of dummy.SetValue("a"); from above
// which essentially does SetModified(false);
// but in fact the current value is "b" so it was modified.

This also applies to all other metric types, as they all seem to call
SetModified after changing the value.
The easy fix would be to put the SetModified into the mutex lock, but
that would result in a deadlock when one of the change listeners calls
SetValue.
I don't see an better way to avoid that race condition though :(

- Jakob

On Tue, 2019-01-22 at 15:49 +0100, Michael Balzer wrote:
> https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/comm
> it/8003fc0da759d5d33682c54cbf38840823477fbc
> 
> Brain surgery. A second pair of eyes on this would be good.
> 
> Regards,
> Michael
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <http://lists.openvehicles.com/pipermail/ovmsdev/attachments/20190123/ef59ce3b/attachment-0002.sig>