Guard update timestamp logic with lock.
Guard the logic to update property value timestamp with lock so that
they will not overlap. Otherwise, it is possible that another op will
update the timestamp to a newer value, which causes this op to fail
the timestamp freshness check.
Test: atest --iterations 10 FakeVehicleHardwareTest#testSubscribe_enableVUR
Bug: 308552957
Change-Id: I8f7a6356ba65e4470e851e59c20bb1f678c2d467
diff --git a/automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp b/automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp
index 7ff03c6..cc4fae1 100644
--- a/automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp
+++ b/automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp
@@ -1055,10 +1055,11 @@
}
auto updatedValue = mValuePool->obtain(value);
- int64_t timestamp = elapsedRealtimeNano();
- updatedValue->timestamp = timestamp;
- auto writeResult = mServerSidePropStore->writeValue(std::move(updatedValue));
+ auto writeResult = mServerSidePropStore->writeValue(
+ std::move(updatedValue),
+ /*updateStatus=*/false, /*mode=*/VehiclePropertyStore::EventMode::ON_VALUE_CHANGE,
+ /*useCurrentTimestamp=*/true);
if (!writeResult.ok()) {
return StatusError(getErrorCode(writeResult))
<< StringPrintf("failed to write value into property store, error: %s",
@@ -2091,10 +2092,10 @@
// Failed to read current value, skip refreshing.
return;
}
- result.value()->timestamp = elapsedRealtimeNano();
- mServerSidePropStore->writeValue(std::move(result.value()), /*updateStatus=*/true,
- eventMode);
+ mServerSidePropStore->writeValue(std::move(result.value()),
+ /*updateStatus=*/true, eventMode,
+ /*useCurrentTimestamp=*/true);
});
mRecurrentTimer->registerTimerCallback(intervalInNanos, action);
mRecurrentActions[propIdAreaId] = action;
diff --git a/automotive/vehicle/aidl/impl/utils/common/include/VehiclePropertyStore.h b/automotive/vehicle/aidl/impl/utils/common/include/VehiclePropertyStore.h
index b74dff5..2812c3b 100644
--- a/automotive/vehicle/aidl/impl/utils/common/include/VehiclePropertyStore.h
+++ b/automotive/vehicle/aidl/impl/utils/common/include/VehiclePropertyStore.h
@@ -100,9 +100,11 @@
// override an existing value, the status for the existing value would be used for the
// overridden value.
// 'EventMode' controls whether the 'OnValueChangeCallback' will be called for this operation.
+ // If 'useCurrentTimestamp' is true, the property value will be set to the current timestamp.
VhalResult<void> writeValue(VehiclePropValuePool::RecyclableType propValue,
bool updateStatus = false,
- EventMode mode = EventMode::ON_VALUE_CHANGE) EXCLUDES(mLock);
+ EventMode mode = EventMode::ON_VALUE_CHANGE,
+ bool useCurrentTimestamp = false) EXCLUDES(mLock);
// Remove a given property value from the property store. The 'propValue' would be used to
// generate the key for the value to remove.
diff --git a/automotive/vehicle/aidl/impl/utils/common/src/VehiclePropertyStore.cpp b/automotive/vehicle/aidl/impl/utils/common/src/VehiclePropertyStore.cpp
index 3fd2aa8..b879850 100644
--- a/automotive/vehicle/aidl/impl/utils/common/src/VehiclePropertyStore.cpp
+++ b/automotive/vehicle/aidl/impl/utils/common/src/VehiclePropertyStore.cpp
@@ -16,6 +16,7 @@
#define LOG_TAG "VehiclePropertyStore"
#include <utils/Log.h>
+#include <utils/SystemClock.h>
#include "VehiclePropertyStore.h"
@@ -107,13 +108,20 @@
VhalResult<void> VehiclePropertyStore::writeValue(VehiclePropValuePool::RecyclableType propValue,
bool updateStatus,
- VehiclePropertyStore::EventMode eventMode) {
+ VehiclePropertyStore::EventMode eventMode,
+ bool useCurrentTimestamp) {
bool valueUpdated = true;
VehiclePropValue updatedValue;
OnValueChangeCallback onValueChangeCallback = nullptr;
{
std::scoped_lock<std::mutex> g(mLock);
+ // Must set timestamp inside the lock to make sure no other writeValue will update the
+ // the timestamp to a newer one while we are writing this value.
+ if (useCurrentTimestamp) {
+ propValue->timestamp = elapsedRealtimeNano();
+ }
+
int32_t propId = propValue->prop;
VehiclePropertyStore::Record* record = getRecordLocked(propId);
