CanvasRenderingContext2D.prototype.drawImage leaks memory

[NAllred91]

YOU MUST read first!

Please use Community Forum for general technical discussions and questions.

• [x] I have used Google with the error message or bug in association with the library and Cordova words to make sure the issue I'm reporting is only related to iOSRTC.
• [ ] I have provided steps to reproduce (e.g. using sample app code https://github.com/cordova-rtc/cordova-plugin-iosrtc-sample or updated extra/renderer-and-libwebrtc-tests.js file).
• [x] I have provided versions of third party library name, ios, Xcode and plugin version and adapter.js version if used.

Note: If the checkboxes above are not checked (which you do after the issue is posted), the issue will be closed, removing this checkbox will result in automatic closed issue.

Versions affected Cordova version (e.g 7.1.0): 10 Cordova iOS version (e.g 5.1.0): 6.2.0 Plugin version (e.g 6.0.12): 6.0.20 iOS version (e.g 10.2): 13 Xcode version (e.g 11.1 - 11A1027): 12 WebRTC-adapter version (e.g. 7.4.0): latest WebRTC Framework version (e.g. JSSip 3.1.2): latest

Description

WebKit leaks memory when changing the src property of an image with data:image/png;base64, strings.

See https://bugs.webkit.org/show_bug.cgi?id=31253

webkit.org bug ticket references a stackoverflow post suggesting to convert the DataURI to a blob. This allows you to manually call revokeObjectURL to free up memory.

See https://stackoverflow.com/a/38624675

I am using drawImage to continuously write a video stream to a canvas. With your patch of drawImage my app will crash after about 10-15 minutes.

I've overwritten your patch in my app using the suggestion in the stackoverflow answer and my app no longer crashes.

This is the code that I believe needs updated to use a Blob and createObjectURL in order to prevent the memory leak. https://github.com/cordova-rtc/cordova-plugin-iosrtc/blob/7179dd51957d609ad0fe95b0ba70ca1c1224ef3e/js/iosrtc.js#L223-L242

[hthetiot]

I will look into it.

[hthetiot]

I've overwritten your patch in my app using the suggestion in the stackoverflow answer and my app no longer crashes.

@NAllred91 can you share your modified version of CanvasRenderingContext2D.prototype.drawImage that you use please :)

[hthetiot]

Should look something like that:

 // Apply CanvasRenderingContext2D.drawImage monkey patch
    var drawImage = CanvasRenderingContext2D.prototype.drawImage;
    CanvasRenderingContext2D.prototype.drawImage = (function () {
        // Methods to address the memory leaks problems in Safari
        let temporaryImage, imageElement;
        const BASE64_MARKER = ';base64,';
        const objectURL = window.URL || window.webkitURL;

        function convertDataURIToBlob(dataURI) {
            // Validate input data
            if (!dataURI) {
                return;
            }

            // Convert image (in base64) to binary data
            const base64Index = dataURI.indexOf(BASE64_MARKER) + BASE64_MARKER.length;
            const base64 = dataURI.substring(base64Index);
            const raw = window.atob(base64);
            const rawLength = raw.length;
            let array = new Uint8Array(new ArrayBuffer(rawLength));

            for (let i = 0; i < rawLength; i++) {
                array[i] = raw.charCodeAt(i);
            }

            // Create and return a new blob object using binary data
            return new Blob([array], { type: 'image/jpeg' });
        }

        return function (arg) {
            var args = Array.prototype.slice.call(arguments);
            var context = this;
            if (arg instanceof HTMLVideoElement && arg.render) {
                arg.render.save(function (base64Image) {
                    // Destroy old image
                    if (temporaryImage) {
                        objectURL.revokeObjectURL(temporaryImage);
                    }

                    // Create a new image from binary data
                    var imageDataBlob = convertDataURIToBlob('data:image/jpg;base64,' + base64Image);

                    // Create a new object URL object
                    imageElement = imageElement || new Image();
                    temporaryImage = objectURL.createObjectURL(imageDataBlob);

                    imageElement.addEventListener('load', function () {
                        args.splice(0, 1, imageElement);
                        drawImage.apply(context, args);
                    });

                    // Set the new image
                    imageElement.src = temporaryImage;
                });
            } else {
                return drawImage.apply(context, args);
            }
        };
    })();

[hthetiot]

Fixed in 6.0.21 and merged on master future 8.0.0

[NAllred91]

This is how I'm currently patching drawImage for myself.

Your changes look good, but I think the URL.revokeObjectURL(temp); call is important as well?

I also have other changes in my code to only create the new window.Image() once, and to promisify drawImage. I've also made changes to prevent drawImage from calling arg.render.save multiple times before the first callback is called, but I removed that from what I've pasted below. I was trying a lot of different things to fix the leak and pretty much stopped touching it once it stopped leaking...

function setupiOSrtc() {
    // Capturing before registerGlobals to get the original
    var drawImage = CanvasRenderingContext2D.prototype.drawImage;

    cordova.plugins.iosrtc.registerGlobals();

    // Methods to address the memory leaks problems in Safari
    const BASE64_MARKER = ';base64,';
    const objectURL = window.URL || window.webkitURL;

    const convertDataURIToBlob = (dataURI: string) => {
        // Validate input data
        if (!dataURI) return;

        // Convert image (in base64) to binary data
        var base64Index = dataURI.indexOf(BASE64_MARKER) + BASE64_MARKER.length;
        var base64 = dataURI.substring(base64Index);
        var raw = window.atob(base64);
        var rawLength = raw.length;
        var array = new Uint8Array(new ArrayBuffer(rawLength));

        for (let i = 0; i < rawLength; i++) {
            array[i] = raw.charCodeAt(i);
        }

        // Create and return a new blob object using binary data
        return new Blob([array], {type: 'image/jpeg'});
    };

    // CanvasRenderingContext2D.prototype.drawImage = drawImage;
    CanvasRenderingContext2D.prototype.drawImage = function (arg) {
        var args = Array.prototype.slice.call(arguments);
        var context = this;
        if (arg instanceof HTMLVideoElement && arg.render) {
            return new Promise<void>((resolve, reject) => {

                var img = this._img || new window.Image();
                this._img = img;

                setTimeout(reject, 500);

                arg.render.save(function (data) {
                    const temp = objectURL.createObjectURL(
                        convertDataURIToBlob('data:image/jpg;base64,' + data)
                    );
                    var onLoad = function () {
                        img.removeEventListener('load', onLoad);
                        args.splice(0, 1, img);
                        drawImage.apply(context, args);
                        img.src = '';
                        URL.revokeObjectURL(temp);
                        resolve();
                    };
                    img.addEventListener('load', onLoad);
                    img.setAttribute('src', temp);
                });
            });
        } else {
            return drawImage.apply(context, args);
        }
    };
}

[hthetiot]

Love it, will apply and let you know.

[hthetiot]

Initial fix is part of 6.0.21 already release, i will make CanvasRenderingContext2D.prototype.drawImage return Promise and apply some other changes you made.