Telemetry leaks memory when dashboard is disabled

[mikemag]

Make a simple opmode which does nothing but FtcDashboard.getInstance().getTelemetry().update(). Disable dashboard with the usual opode, then run the test opmode. Observe GC messages in logcat to see the system running out of memory. The DS will eventually disconnect (no heartbeat) as GC's take longer and longer, and the RC will eventually be killed and restarted.

sendTelemetryPacket adds a packet to the pendingTelemetry list unconditionally: https://github.com/acmerobotics/ftc-dashboard/blob/a170c186e4deb7f44c6102fc76572074a200289a/FtcDashboard/src/main/java/com/acmerobotics/dashboard/FtcDashboard.java#L685-L693

It relies on the thread servicing TelemetryUpdateRunnable to a) keep up and b) actually exist: https://github.com/acmerobotics/ftc-dashboard/blob/a170c186e4deb7f44c6102fc76572074a200289a/FtcDashboard/src/main/java/com/acmerobotics/dashboard/FtcDashboard.java#L237

TelemetryUpdateRunnable is only started when dashboard is enabled. Thus pendingTelemetry grows without bound.

Sample opmode:

package org.firstinspires.ftc.teamcode.opmodes;

import com.acmerobotics.dashboard.FtcDashboard;
import com.qualcomm.robotcore.eventloop.opmode.LinearOpMode;
import com.qualcomm.robotcore.eventloop.opmode.TeleOp;

@TeleOp(name = "Dash Test")
public class BasicOpMode_Linear extends LinearOpMode {
    @Override
    public void runOpMode() {
        waitForStart();

        while (opModeIsActive()) {
            FtcDashboard.getInstance().getTelemetry().update();
        }
    }
}

Note also that TelemetryPacker.Adapter.update() unconditionally adds an empty packet in this example, even though nothing has been added to it.

[rbrott]

Yikes, looks like this has been lurking for a while. I'll have to audit the rest of the backend to make sure it handles enable/disable states properly.

Thanks for the detailed report.

[mikemag]

Would you consider a new point release with this fix? We had multiple teams at the qualification match yesterday crashing mid-game, and each one had dutifully disabled dashboard. Explaining this to them, and having them re-enable, or remove dashboard telemetry, solved the problems. A new release would go a long way towards preempting this.