Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f8417611 authored by Yu Shan's avatar Yu Shan
Browse files

Make refreshTimestamp atomic. 

Make refreshTimestamp atomic in VehiclePropertyStore so that we
are sure the property value will not be updated after we read
the value out, but before we put the value with the new timestamp back.

Otherwise there is a slight chance that a property update will be
overwritten by the timestamp refresh operation.

Test: atest FakeVehicleHardwareTest
Bug: 308552957
Change-Id: I789257b4c75d8a4d19edeec31d01dc23776233ec
parent 5ba717d1

automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp

+4 −19
Original line number Diff line number Diff line
@@ -2078,24 +2078,9 @@ StatusCode FakeVehicleHardware::subscribePropIdAreaIdLocked( 
                enableVariableUpdateRate) {

                eventMode = VehiclePropertyStore::EventMode::ON_VALUE_CHANGE;

            }

            auto action = std::make_shared<RecurrentTimer::Callback>([this, propId, areaId,

                                                                      eventMode] {

                // Refresh the property value. In real implementation, this should poll the latest

                // value from vehicle bus. Here, we are just refreshing the existing value with a

                // new timestamp.

                auto result = getValue(VehiclePropValue{

                        .areaId = areaId,

                        .prop = propId,

                        .value = {},

                });

                if (!result.ok()) {

                    // Failed to read current value, skip refreshing.

                    return;

                }



                mServerSidePropStore->writeValue(std::move(result.value()),

                                                 /*updateStatus=*/true, eventMode,

                                                 /*useCurrentTimestamp=*/true);

            auto action =

                    std::make_shared<RecurrentTimer::Callback>([this, propId, areaId, eventMode] {

                        mServerSidePropStore->refreshTimestamp(propId, areaId, eventMode);

                    });

            mRecurrentTimer->registerTimerCallback(intervalInNanos, action);

            mRecurrentActions[propIdAreaId] = action;

automotive/vehicle/aidl/impl/utils/common/include/VehiclePropertyStore.h

+5 −0
Original line number Diff line number Diff line
@@ -106,6 +106,11 @@ class VehiclePropertyStore final { 
                                EventMode mode = EventMode::ON_VALUE_CHANGE,

                                bool useCurrentTimestamp = false) EXCLUDES(mLock);



    // Refresh the timestamp for the stored property value for [propId, areaId]. If eventMode is

    // always, generates the property update event, otherwise, only update the stored timestamp

    // without generating event. This operation is atomic with other writeValue operations.

    void refreshTimestamp(int32_t propId, int32_t areaId, EventMode eventMode) 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.

    void removeValue(

automotive/vehicle/aidl/impl/utils/common/src/VehiclePropertyStore.cpp

+36 −0
Original line number Diff line number Diff line
@@ -176,6 +176,42 @@ VhalResult<void> VehiclePropertyStore::writeValue(VehiclePropValuePool::Recyclab 
    return {};

}



void VehiclePropertyStore::refreshTimestamp(int32_t propId, int32_t areaId, EventMode eventMode) {

    VehiclePropValue updatedValue;

    OnValueChangeCallback onValueChangeCallback = nullptr;

    {

        std::scoped_lock<std::mutex> g(mLock);



        VehiclePropertyStore::Record* record = getRecordLocked(propId);

        if (record == nullptr) {

            return;

        }



        VehiclePropValue propValue = {

                .areaId = areaId,

                .prop = propId,

                .value = {},

        };



        VehiclePropertyStore::RecordId recId = getRecordIdLocked(propValue, *record);

        if (auto it = record->values.find(recId); it != record->values.end()) {

            it->second->timestamp = elapsedRealtimeNano();

            updatedValue = *(it->second);

        } else {

            return;

        }

        if (!mOnValueChangeCallback) {

            return;

        }

        onValueChangeCallback = mOnValueChangeCallback;

    }



    // Invoke the callback outside the lock to prevent dead-lock.

    if (eventMode == EventMode::ALWAYS) {

        onValueChangeCallback(updatedValue);

    }

}



void VehiclePropertyStore::removeValue(const VehiclePropValue& propValue) {

    std::scoped_lock<std::mutex> g(mLock);