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

Commit 69f22feb authored by Mathias Agopian's avatar Mathias Agopian
Browse files

fix [2476230] sensor battery stats could get out of sync if an error occurs

Fixed a few problems with the SensorService:
- a race condition when talking to the BatteryStatService
- only report changes to BatteryStatService when there are no errors
(ie: when a change actually happens)
- tell the BatteryStatService when a sensor is deactivated because its
client died
- rewrite enableSensor() so it's readable
- implement dump() so dumpsys will display some infos about active sensors
- recompute the delay properly when sensors are added/removed
parent 42c79880
Loading
Loading
Loading
Loading
+134 −58
Original line number Diff line number Diff line
@@ -24,7 +24,11 @@ import android.os.RemoteException;
import android.os.IBinder;
import android.util.Config;
import android.util.Slog;
import android.util.PrintWriterPrinter;
import android.util.Printer;

import java.io.FileDescriptor;
import java.io.PrintWriter;
import java.util.ArrayList;

import com.android.internal.app.IBatteryStats;
@@ -43,6 +47,7 @@ class SensorService extends ISensorService.Stub {
    private static final boolean DEBUG = false;
    private static final boolean localLOGV = DEBUG ? Config.LOGD : Config.LOGV;
    private static final int SENSOR_DISABLE = -1;
    private int mCurrentDelay = 0;
    
    /**
     * Battery statistics to be updated when sensors are enabled and disabled.
@@ -51,17 +56,19 @@ class SensorService extends ISensorService.Stub {

    private final class Listener implements IBinder.DeathRecipient {
        final IBinder mToken;
        final int mUid;

        int mSensors = 0;
        int mDelay = 0x7FFFFFFF;
        
        Listener(IBinder token) {
        Listener(IBinder token, int uid) {
            mToken = token;
            mUid = uid;
        }
        
        void addSensor(int sensor, int delay) {
            mSensors |= (1<<sensor);
            if (mDelay > delay)
            if (delay < mDelay)
            	mDelay = delay;
        }
        
@@ -83,16 +90,20 @@ class SensorService extends ISensorService.Stub {
                for (int sensor=0 ; sensor<32 && mSensors!=0 ; sensor++) {
                    if (hasSensor(sensor)) {
                        removeSensor(sensor);
                        try {
                        deactivateIfUnusedLocked(sensor);
                        try {
                            mBatteryStats.noteStopSensor(mUid, sensor);
                        } catch (RemoteException e) {
                            Slog.w(TAG, "RemoteException in binderDied");
                            // oops. not a big deal.
                        }
                    }
                }
                if (mListeners.size() == 0) {
                    _sensors_control_wake();
                    _sensors_control_close();
                } else {
                    // TODO: we should recalculate the delay, since removing
                    // a listener may increase the overall rate.
                }
                mListeners.notify();
            }
@@ -114,10 +125,26 @@ class SensorService extends ISensorService.Stub {

    public boolean enableSensor(IBinder binder, String name, int sensor, int enable)
            throws RemoteException {
        
        if (localLOGV) Slog.d(TAG, "enableSensor " + name + "(#" + sensor + ") " + enable);

        // Inform battery statistics service of status change
        if (binder == null) {
            Slog.e(TAG, "listener is null (sensor=" + name + ", id=" + sensor + ")");
            return false;
        }

        if (enable < 0 && (enable != SENSOR_DISABLE)) {
            Slog.e(TAG, "invalid enable parameter (enable=" + enable +
                    ", sensor=" + name + ", id=" + sensor + ")");
            return false;
        }

        boolean res;
        int uid = Binder.getCallingUid();
        synchronized(mListeners) {
            res = enableSensorInternalLocked(binder, uid, name, sensor, enable);
            if (res == true) {
                // Inform battery statistics service of status change
                long identity = Binder.clearCallingIdentity();
                if (enable == SENSOR_DISABLE) {
                    mBatteryStats.noteStopSensor(uid, sensor);
@@ -125,74 +152,123 @@ class SensorService extends ISensorService.Stub {
                    mBatteryStats.noteStartSensor(uid, sensor);
                }
                Binder.restoreCallingIdentity(identity);

        if (binder == null) {
            Slog.w(TAG, "listener is null (sensor=" + name + ", id=" + sensor + ")");
            return false;
            }

        synchronized(mListeners) {
            if (enable!=SENSOR_DISABLE && !_sensors_control_activate(sensor, true)) {
                Slog.w(TAG, "could not enable sensor " + sensor);
                return false;
        }
        return res;
    }

    private boolean enableSensorInternalLocked(IBinder binder, int uid,
            String name, int sensor, int enable) throws RemoteException {

        // check if we have this listener
        Listener l = null;
            int minDelay = enable;
        for (Listener listener : mListeners) {
            if (binder == listener.mToken) {
                l = listener;
                break;
            }
                if (minDelay > listener.mDelay)
                    minDelay = listener.mDelay;
        }

            if (l == null && enable!=SENSOR_DISABLE) {
                l = new Listener(binder);
        if (enable != SENSOR_DISABLE) {
            // Activate the requested sensor
            if (_sensors_control_activate(sensor, true) == false) {
                Slog.w(TAG, "could not enable sensor " + sensor);
                return false;
            }

            if (l == null) {
                /*
                 * we don't have a listener for this binder yet, so
                 * create a new one and add it to the list.
                 */
                l = new Listener(binder, uid);
                binder.linkToDeath(l, 0);
                mListeners.add(l);
                mListeners.notify();
            }

            // take note that this sensor is now used by this client
            l.addSensor(sensor, enable);

        } else {

            if (l == null) {
                // by construction, this means we're disabling a listener we
                // don't know about...
                /*
                 *  This client isn't in the list, this usually happens
                 *  when enabling the sensor failed, but the client
                 *  didn't handle the error and later tries to shut that
                 *  sensor off.
                 */
                Slog.w(TAG, "listener with binder " + binder +
                        ", doesn't exist (sensor=" + name + ", id=" + sensor + ")");
                        ", doesn't exist (sensor=" + name +
                        ", id=" + sensor + ")");
                return false;
            }

            if (minDelay >= 0) {
                _sensors_control_set_delay(minDelay);
            }
            
            if (enable != SENSOR_DISABLE) {
                l.addSensor(sensor, enable);
            } else {
            // remove this sensor from this client
            l.removeSensor(sensor);

            // see if we need to deactivate this sensors=
            deactivateIfUnusedLocked(sensor);

            // if the listener doesn't have any more sensors active
            // we can get rid of it
            if (l.mSensors == 0) {
                    mListeners.remove(l);
                // we won't need this death notification anymore
                binder.unlinkToDeath(l, 0);
                // remove the listener from the list
                mListeners.remove(l);
                // and if the list is empty, turn off the whole sensor h/w
                if (mListeners.size() == 0) {
                    _sensors_control_wake();
                    _sensors_control_close();
                }
                mListeners.notify();
            }
        }

            if (mListeners.size() == 0) {
                _sensors_control_wake();
                _sensors_control_close();
        // calculate and set the new delay
        int minDelay = 0x7FFFFFFF;
        for (Listener listener : mListeners) {
            if (listener.mDelay < minDelay)
                minDelay = listener.mDelay;
        }
        if (minDelay != 0x7FFFFFFF) {
            mCurrentDelay = minDelay;
            _sensors_control_set_delay(minDelay);
        }

        return true;
    }

    private void deactivateIfUnusedLocked(int sensor) throws RemoteException {
    private void deactivateIfUnusedLocked(int sensor) {
        int size = mListeners.size();
        for (int i=0 ; i<size ; i++) {
            if (mListeners.get(i).hasSensor(sensor))
            if (mListeners.get(i).hasSensor(sensor)) {
                // this sensor is still in use, don't turn it off
                return;
            }
        _sensors_control_activate(sensor, false);
        }
        if (_sensors_control_activate(sensor, false) == false) {
            Slog.w(TAG, "could not disable sensor " + sensor);
        }
    }

    @Override
    protected void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
        synchronized (mListeners) {
            Printer pr = new PrintWriterPrinter(pw);
            int c = 0;
            pr.println(mListeners.size() + " listener(s), delay=" + mCurrentDelay + " ms");
            for (Listener l : mListeners) {
                pr.println("listener[" + c + "] " +
                        "sensors=0x" + Integer.toString(l.mSensors, 16) +
                        ", uid=" + l.mUid +
                        ", delay=" +
                        l.mDelay + " ms");
                c++;
            }
        }
    }

    private ArrayList<Listener> mListeners = new ArrayList<Listener>();