zathras / jovial_svg

Flutter library for robust, efficient rendering of SVG static images
BSD 3-Clause "New" or "Revised" License
113 stars 21 forks source link

Native Crash: `_NativeCanvas.restore` throws EXC_BAD_ACCESS (KERN_INVALID_ADDRESS) #78

Closed Sese-Schneider closed 11 months ago

Sese-Schneider commented 12 months ago

Details

Around 1% of our iOS app instances are crashing when viewing large/complex SVGs.

Crashed: io.flutter.1.ui EXC_BAD_ACCESS KERN_INVALID_ADDRESS 0x000000011daf7fc4

Crash log ``` Crashed: io.flutter.1.ui 0 Flutter 0x33b974 (Missing UUID 4c4c441f55553144a1682f45551c1451) 1 App 0xc9668 FfiTrampoline___pop$Method$FfiNative$Ptr + 1 (dart:ffi:1) 2 App 0x9e30cc _NativeCanvas.restore + 5801 (painting.dart:5801) 3 App 0xa84834 SIGroupHelper.endPaintGroup + 454 (common.dart:454) 4 App 0xa84700 SIGroup.paint + 276 (array.dart:276) 5 App 0xa84784 SIGroup.paint + 430 (dag.dart:430) 6 App 0xa84784 SIGroup.paint + 430 (dag.dart:430) 7 App 0xa84784 SIGroup.paint + 430 (dag.dart:430) 8 App 0x5c7a68 ScalableImageBase.paint + 812 (exported.dart:812) 9 App 0x5c7854 _SIPainter.paint + 285 (widget.dart:285) 10 App 0x259540 RenderCustomPaint._paintWithPainter + 609 (custom_paint.dart:609) 11 App 0x259234 RenderCustomPaint.paint + 616 (custom_paint.dart:616) 12 App 0xc226c RenderObject._paintWithContext + 3159 (object.dart:3159) 13 App 0xa4efa8 PaintingContext.paintChild + 236 (object.dart:236) 14 App 0x262498 RenderProxyBoxMixin.paint + 123 (proxy_box.dart:123) 15 App 0xc226c RenderObject._paintWithContext + 3159 (object.dart:3159) 16 App 0xc21b4 PaintingContext._repaintCompositedChild + 173 (object.dart:173) 17 App 0xc1dc4 PipelineOwner.flushPaint + 192 (object.dart:192) 18 App 0xb8728 RendererBinding.drawFrame + 498 (binding.dart:498) 19 App 0xb7fb8 WidgetsBinding.drawFrame + 919 (binding.dart:919) 20 App 0xb7984 RendererBinding._handlePersistentFrameCallback + 361 (binding.dart:361) 21 App 0xb794c RendererBinding._handlePersistentFrameCallback + 359 (binding.dart:359) 22 App 0x74738 SchedulerBinding._invokeFrameCallback + 558 (growable_array.dart:558) 23 App 0x74668 SchedulerBinding.handleDrawFrame + 338 (iterable.dart:338) 24 App 0x74380 SchedulerBinding._handleDrawFrame + 1067 (binding.dart:1067) 25 App 0x74258 SchedulerBinding._handleDrawFrame + 1067 (binding.dart:1067) 26 App 0x2a758 _rootRun + 1394 (zone.dart:1394) 27 App 0x2a84c _rootRun + 1390 (zone.dart:1390) 28 App 0x9e3db0 _CustomZone.run + 1297 (zone.dart:1297) 29 App 0x9e4a88 _CustomZone.runGuarded + 1211 (zone.dart:1211) 30 App 0x50150 _invoke + 165 (hooks.dart:165) 31 App 0x50098 PlatformDispatcher._drawFrame + 400 (platform_dispatcher.dart:400) 32 App 0x50054 _drawFrame + 138 (hooks.dart:138) 33 App 0x50180 _drawFrame + 138 (hooks.dart:138) 34 App 0xbc94 stub InvokeDartCode + 31124 35 Flutter 0x61b920 (Missing UUID 4c4c441f55553144a1682f45551c1451) 36 Flutter 0x7296d0 (Missing UUID 4c4c441f55553144a1682f45551c1451) 37 Flutter 0x3e75a0 (Missing UUID 4c4c441f55553144a1682f45551c1451) 38 Flutter 0x3c9990 (Missing UUID 4c4c441f55553144a1682f45551c1451) 39 Flutter 0x3f97bc (Missing UUID 4c4c441f55553144a1682f45551c1451) 40 Flutter 0x2de710 (Missing UUID 4c4c441f55553144a1682f45551c1451) 41 Flutter 0x2e1d00 (Missing UUID 4c4c441f55553144a1682f45551c1451) 42 CoreFoundation 0xcb624 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ + 32 43 CoreFoundation 0x88f4c __CFRunLoopDoTimer + 940 44 CoreFoundation 0x2e71c __CFRunLoopDoTimers + 288 45 CoreFoundation 0x7a4d4 __CFRunLoopRun + 1852 46 CoreFoundation 0x7f3ec CFRunLoopRunSpecific + 612 47 Flutter 0x2e1dec (Missing UUID 4c4c441f55553144a1682f45551c1451) 48 Flutter 0x2e1460 (Missing UUID 4c4c441f55553144a1682f45551c1451) 49 libsystem_pthread.dylib 0x16b8 _pthread_start + 148 50 libsystem_pthread.dylib 0xb88 thread_start + 8 ```

Additional context

  • Have you narrowed it down to a reasonably concise and small SVG file that reproduces the issue?

We've discovered that SVGs causing this issues have to be rather complex, which causes the file size.

We've also filed a bug report with the flutter team, as we're not sure if the issue is an implementation glitch from jovial_svg or a general dart iOS issue: https://github.com/flutter/flutter/issues/138607

Sese-Schneider commented 12 months ago

How to reproduce

We've managed to create a minimal reproduceable sample app which crashes in iOS

import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
import 'package:jovial_svg/jovial_svg.dart';

void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  // This widget is the root of your application.
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'SVG rendering crash Demo',
      home: Scaffold(
        body: CustomScrollView(
          slivers: [
            const SliverAppBar(
              title: Text('SVG rendering crash Demo'),
            ),
            SliverList.builder(
              itemBuilder: (context, index) => SizedBox(
                key: ValueKey(index),
                height: 300,
                child: (index % 4 == 0)
                    ? _buildSVGWidget(index)
                    : ColoredBox(
                        color: Color.fromARGB(
                            0xff, 64 * ((index % 4) + 1), 0x88, 0xaa)),
              ),
            ),
          ],
        ),
      ),
    );
  }

  ScalableImageWidget _buildSVGWidget(int index) {
    final assetIndex = 2;
    final svgSource =
        ScalableImageSource.fromSvg(rootBundle, 'assets/test$assetIndex.svg');
    return ScalableImageWidget.fromSISource(
      si: svgSource,
      fit: BoxFit.contain,
      isComplex: true,
    );
  }
}

Asset we've used: test1.svg

zathras commented 12 months ago

Neat image!

Yes, I think this has to be a platform bug. jovial_svg doesn't have any native code, so there's certainly a platform bug at work.

With that said, there's a small possibility that jovial_svg is doing something wrong that triggers a platform bug. Notably, _NativeCanvas.restore is interesting. There's an outside possibility that jovial_svg has unbalanced calls to Canvas.save()/saveLayer() and Canvas.restore(). If it does, that shouldn't cause a memory error like you're seeing, so that's why I say there's definitely at least a platform bug. But jovial_svg shouldn't have unbalanced calls to save/restore, so maybe it does and that's triggering the platform bug?

I'm fairly confident that the calls are balanced in jovial_svg, like they're supposed to be, but I'll check it out with that asset just to be sure. Probably this weekend?

One thing that would be a little bit interesting if you tried on your end: You can use the "compact" internal representation of the ScalableImage. That should result in the exact same underlying calls to Canvas, but with an internal structure that's a lot more memory-efficient (and substantially less time-efficient). The expected result would be that you'd see the same problem. If you don't, it's evidence that the platform bug is more likely related to general memory pressure issues, and not the Canvas API.

billf-pshs commented 11 months ago

Well, the good news is that jovial_svg properly balances save/restore calls on that asset. But from your perspective, I guess that's the bad news too, in that there's no easy fix on my end; whatever the bug is in the platform native code needs to be fixed.

In case it's of any use, here's the little debug counter class I used before all calls to save, saveLayer or restore:

class DB {
  final int canvasID;
  int saveCount = 0;

  DB(this.canvasID);

  static final _saved = <Canvas, DB>{};

  static DB _get(Canvas c) {
      return _saved.putIfAbsent(c, () => new DB(_saved.length + 1) );
  }

  static saveOrLayer(Canvas c) {
    final db = _get(c);
    db.saveCount++;
    print('save     ${db.canvasID}  ${db.saveCount}');
  }

  static restore(Canvas c) {
    final db = _get(c);
    db.saveCount--;
    print('restore  ${db.canvasID}  ${db.saveCount}');
  }
}

And here are the results on your asset:

flutter: save     19  1
flutter: save     19  2
flutter: save     19  3
flutter: save     19  4
flutter: restore  19  3
flutter: save     19  4
flutter: save     19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: save     19  9
flutter: save     19  10
flutter: save     19  11
flutter: restore  19  10
flutter: save     19  11
flutter: save     19  12
flutter: restore  19  11
flutter: restore  19  10
flutter: restore  19  9
flutter: restore  19  8
flutter: save     19  9
flutter: restore  19  8
flutter: save     19  9
flutter: restore  19  8
flutter: save     19  9
flutter: restore  19  8
flutter: save     19  9
flutter: restore  19  8
flutter: save     19  9
flutter: restore  19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: save     19  6
flutter: restore  19  5
flutter: restore  19  4
flutter: restore  19  3
flutter: restore  19  2
flutter: save     19  3
flutter: save     19  4
flutter: restore  19  3
flutter: save     19  4
flutter: save     19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: save     19  9
flutter: restore  19  8
flutter: save     19  9
flutter: restore  19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: save     19  6
flutter: save     19  7
flutter: restore  19  6
flutter: save     19  7
flutter: save     19  8
flutter: save     19  9
flutter: save     19  10
flutter: restore  19  9
flutter: save     19  10
flutter: restore  19  9
flutter: restore  19  8
flutter: save     19  9
flutter: restore  19  8
flutter: restore  19  7
flutter: restore  19  6
flutter: restore  19  5
flutter: restore  19  4
flutter: restore  19  3
flutter: restore  19  2
flutter: restore  19  1
flutter: restore  19  0
zathras commented 11 months ago

Closing this, as it seems pretty clear it's a platform bug (https://github.com/flutter/flutter/issues/138607), and it's unlikely jovial_svg can do anything to work around it.

billf-pshs commented 11 months ago

Hi Sebastian,

On a lark, I looked at the source SVG to see if anything jumped out at me. The main thing that I noticed is that it contains some very long paths. jovial_svg translates these into Flutter objects of type dart:ui:Path. It's a shot in the dark, but I wonder if the native side of the Flutter implementation might have a memory corruption bug that's triggered by unexpected long paths?

I guess the corruption bug you're seeing is hard to reproduce, but if this is important enough to you, you could artificially limit the length of the paths, and see if that fixes the crash. It would be easy to add an artificial limit by hacking jovial_svg's source. For example, in lib/src/path.dart, you could just add a counter to each UIPathBuilder instance, and once it reaches some limit (like 50 path elements), just start ignoring subsequent calls (except end).

Obviously that would mess up the appearance of the assets, but if it makes the crash go away, it might help the Flutter team to narrow down the problem. You might even be able to reproduce it in a stand-alone program (not using jovial_svg) by just trying to draw an absurdly long path.

It might also point to a workaround, if you can modify asset creation to avoid creating really long paths.

Anyway, just a thought.

Sese-Schneider commented 11 months ago

@billf-pshs thank you for your suggestion! We are indeed going for a similar approach right now, not adjusting the source code but limiting what our design team is allowed to upload as an asset. I think its worth exploring your idea though, I will do that!

billf-pshs commented 11 months ago

Cool! Let me know how it turns out.

I actually counted the path length in your asset, and one of the paths had a little over 6000 operations on it. That's not insane or anything -- the Ukraine map in the jovial_svg demo program has a path with maybe 4500 operations -- but it is pretty big.

I thought maybe one interesting test might be to draw a circle like this:

import dart:ui
...
    final p = Path(); (or however one creates a Path in Flutter - don't have that memorized :-) )
    final N = 100000;
    for i in 0..N-1
        calculate the point at 2*pi*i/N around the circle
        add it to the path
    close the path
    draw it

If that blows up on those iOS platforms, then it's a pretty good smoking gun. Maybe even try dividing the circle up into a million segments, to really push the library. I'd say refusing to draw the shape (or throwing some kind of exception) is fair for a truly excessive path, but corrupting memory and crashing isn't.