Skip to content

Refactor tonic's handling of typed data #99431

@dnfield

Description

@dnfield

The code in //third_party/tonic/typed_data/typed_list.h is dangerous as written.

The underlying C API it wraps says that you cannot do anything that would cause an allocation to occur before releasing the data, but for users of e.g. a Float32List object, that's not at all clear. In fact, it's not always clear what other methods in the engine may go down some branch that will end up causing a Dart allocation to happen. It is also difficult because tonic wrapped C++ methods make the object look like something scoped to your method, and give no indication that you would ever need to release it early. I and other engine developers have made this mistake (see flutter/engine#31745 and related issues).

In general, the usage of typed data needs to follow an Acquire - Copy - Release pattern. Ideally, Tonic would just give the method the copied result (but we have to evaluate whether this can work for Skia/DL APIs that we pass the data into and end up doing copying behind the scenes).

For example, instead of a method taking a const tonic::Float64List& matrix4 parameter, it would just take a SkM44 parameter. Instead of taking a const tonic::Uint8List& data, it would take whatever structure that data gets copied into.

The main goal here is to make it harder for contributors to make mistakes on this, particularly contributors who are not familiar with the underlying Dart API that is getting wrapped and assume this is a RAII object like any other.

/cc @flar for thoughts on whether/how this works with display list code.

/cc @zanderso @chinmaygarde @jason-simmons

Metadata

Metadata

Assignees

No one assigned

    Labels

    P2Important issues not at the top of the work listc: tech-debtTechnical debt, code quality, testing, etc.engineflutter/engine related. See also e: labels.team-engineOwned by Engine teamtriaged-engineTriaged by Engine team

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions