cppfw / svgren

:camera: SVG rendering library in C++
MIT License
206 stars 41 forks source link

Multithreading problems #54

Closed Papirosnik closed 6 years ago

Papirosnik commented 6 years ago

Besides this issue (https://github.com/igagis/svgren/issues/51) I bumped into crash because of absence of thread-safe in cairo. I wanted to render some svg assets asynchronously and failed.

Skia (https://skia.org/) looks like a good alternative to cairo library. At least they state that it is thread-safe and has better performance...

Any chances to taste it?

igagis commented 6 years ago

I suppose you tried to call svgren::render() from two different threads? I.e. render two different SVGs in parallel?

igagis commented 6 years ago

According to this SO thread the cairo should be reentrant...

igagis commented 6 years ago

Transition to SKIA is effort-wise expensive... so I'd like to avoid that.

igagis commented 6 years ago

By the way, can you share the images you tried to render in parallel? I could try to make a test app which does parallel rendering and try to analyse the problem, maybe it is easily fixable...

Papirosnik commented 6 years ago

I cannot share them for public access, so will be ok to send them for you by email?

igagis commented 6 years ago

yes, by email is ok

igagis commented 6 years ago

Ok, I received the files, thanks. One more question: which OS did you run? Windows/Android/Somethingelse?

Papirosnik commented 6 years ago

I tried on android and osx and got crashes on both platforms in multithreaded rendering case. Simultaneous render does not crash.

Papirosnik commented 6 years ago

Locking of entire render function helps but it is quite expensive... It would be nice if you could enter lock somewhere inside lib more precisely.

igagis commented 6 years ago

I suppose your crash happens in cairo_debug_reset_static_data function, right?

Papirosnik commented 6 years ago

Nope, I'm not sure Last available stacktrace: lib/arm/libsvgren.so (fast_composite_over_8888_8888+432) lib/arm/libsvgren.so (pixman_image_composite32+1432) lib/arm/libsvgren.so (_inplace_spans+648) lib/arm/libsvgren.so (blit_a8+442) lib/arm/libsvgren.so (glitter_scan_converter_render+552) lib/arm/libsvgren.so (_cairo_tor_scan_converter_generate+104) lib/arm/libsvgren.so (composite_polygon+722) lib/arm/libsvgren.so (clip_and_composite_polygon+442) lib/arm/libsvgren.so (_cairo_spans_compositor_fill+600) lib/arm/libsvgren.so (_cairo_compositor_fill+162) lib/arm/libsvgren.so (_cairo_image_surface_fill+54) lib/arm/libsvgren.so (_cairo_surface_fill+170) lib/arm/libsvgren.so (_cairo_gstate_fill+548) lib/arm/libsvgren.so (_cairo_default_context_fill_preserve+26) lib/arm/libsvgren.so (cairo_fill_preserve+28) lib/arm/libsvgren.so (_ZN6svgren8Renderer18renderCurrentShapeEb+380) lib/arm/libsvgren.so (_ZN6svgren8Renderer5visitERKN6svgdom11PathElementE+3046) lib/arm/libsvgren.so (_ZNK6svgdom11PathElement6acceptERNS_12ConstVisitorE+36) lib/arm/libsvgren.so (_ZN6svgdom12ConstVisitor11relayAcceptERKNS_9ContainerE+388) lib/arm/libsvgren.so (_ZN6svgren8Renderer5visitERKN6svgdom8GElementE+132) lib/arm/libsvgren.so (_ZNK6svgdom8GElement6acceptERNS_12ConstVisitorE+36) lib/arm/libsvgren.so (_ZN6svgdom12ConstVisitor11relayAcceptERKNS_9ContainerE+388) lib/arm/libsvgren.so (_ZN6svgren8Renderer5visitERKN6svgdom8GElementE+132) lib/arm/libsvgren.so (_ZNK6svgdom8GElement6acceptERNS_12ConstVisitorE+36) lib/arm/libsvgren.so (_ZN6svgdom12ConstVisitor11relayAcceptERKNS_9ContainerE+388) lib/arm/libsvgren.so (_ZN6svgren8Renderer5visitERKN6svgdom8GElementE+132) lib/arm/libsvgren.so (_ZNK6svgdom8GElement6acceptERNS_12ConstVisitorE+36) lib/arm/libsvgren.so (_ZN6svgdom12ConstVisitor11relayAcceptERKNS_9ContainerE+388) lib/arm/libsvgren.so (_ZN6svgren8Renderer5visitERKN6svgdom8GElementE+132) lib/arm/libsvgren.so (_ZNK6svgdom8GElement6acceptERNS_12ConstVisitorE+36) lib/arm/libsvgren.so (_ZN6svgdom12ConstVisitor11relayAcceptERKNS_9ContainerE+388) lib/arm/libsvgren.so (_ZN6svgren8Renderer5visitERKN6svgdom8GElementE+132) lib/arm/libsvgren.so (_ZNK6svgdom8GElement6acceptERNS_12ConstVisitorE+36) lib/arm/libsvgren.so (_ZN6svgdom12ConstVisitor11relayAcceptERKNS_9ContainerE+388) lib/arm/libsvgren.so (_ZN6svgren8Renderer5visitERKN6svgdom8GElementE+132) lib/arm/libsvgren.so (_ZNK6svgdom8GElement6acceptERNS_12ConstVisitorE+36) lib/arm/libsvgren.so (_ZN6svgdom12ConstVisitor11relayAcceptERKNS_9ContainerE+388) lib/arm/libsvgren.so (_ZN6svgren8Renderer16renderSvgElementERKN6svgdom9ContainerERKNS1_9StyleableERKNS1_9ViewBoxedERKNS1_13AspectRatioedERKNS1_6LengthESG_SGSG+406) lib/arm/libsvgren.so (_ZN6svgren8Renderer5visitERKN6svgdom10SvgElementE+50) lib/arm/libsvgren.so (_ZNK6svgdom10SvgElement6acceptERNS_12ConstVisitorE+36) lib/arm/libsvgren.so (Render+346)

igagis commented 6 years ago

Ok. I reproduced the crash on linux and it seems because of that cairo_debug_reset_static_data(). I removed call to cairo_debug_reset_static_data() in version 0.4.52. Please try if it fixes your crash. The version should be released soon: https://travis-ci.org/igagis/svgren/builds/371127896 https://ci.appveyor.com/project/igagis/svgren/build/master-511

Papirosnik commented 6 years ago

No, it doesn't help. Still crashes. Native stacktrace: ... 2 libsvgren.dylib 0x0000000118ef445d fast_composite_over_8888_8888 + 128 3 libsvgren.dylib 0x0000000118ee712f pixman_image_composite32 + 999 4 libsvgren.dylib 0x0000000118f5f8a6 _inplace_spans + 272 5 libsvgren.dylib 0x0000000118f34470 _cairo_tor_scan_converter_generate + 3338 6 libsvgren.dylib 0x0000000118f3dac5 composite_polygon + 328 7 libsvgren.dylib 0x0000000118f3d830 clip_and_composite_polygon + 343 8 libsvgren.dylib 0x0000000118f3c9a6 _cairo_spans_compositor_fill + 508 9 libsvgren.dylib 0x0000000118f36b71 _cairo_compositor_fill + 110 10 libsvgren.dylib 0x0000000118f38d2b _cairo_image_surface_fill + 41 11 libsvgren.dylib 0x0000000118f64edc _cairo_surface_fill + 174 12 libsvgren.dylib 0x0000000118f43aff _cairo_gstate_fill + 377 13 libsvgren.dylib 0x0000000118f3fc4d cairo_fill_preserve + 28 14 libsvgren.dylib 0x0000000118ecc3d7 _ZN6svgren8Renderer18renderCurrentShapeEb + 539 15 libsvgren.dylib 0x0000000118ecd9e0 _ZN6svgren8Renderer5visitERKN6svgdom11PathElementE + 2578 16 libsvgren.dylib 0x0000000118f853fb _ZN6svgdom12ConstVisitor11relayAcceptERKNS_9ContainerE + 35 17 libsvgren.dylib 0x0000000118eccb64 _ZN6svgren8Renderer5visitERKN6svgdom8GElementE + 162 18 libsvgren.dylib 0x0000000118f853fb _ZN6svgdom12ConstVisitor11relayAcceptERKNS_9ContainerE + 35 19 libsvgren.dylib 0x0000000118eccb64 _ZN6svgren8Renderer5visitERKN6svgdom8GElementE + 162 20 libsvgren.dylib 0x0000000118f853fb _ZN6svgdom12ConstVisitor11relayAcceptERKNS_9ContainerE + 35 21 libsvgren.dylib 0x0000000118eccb64 _ZN6svgren8Renderer5visitERKN6svgdom8GElementE + 162 22 libsvgren.dylib 0x0000000118f853fb _ZN6svgdom12ConstVisitor11relayAcceptERKNS_9ContainerE + 35 23 libsvgren.dylib 0x0000000118eccb64 _ZN6svgren8Renderer5visitERKN6svgdom8GElementE + 162 24 libsvgren.dylib 0x0000000118f853fb _ZN6svgdom12ConstVisitor11relayAcceptERKNS_9ContainerE + 35 25 libsvgren.dylib 0x0000000118ecc851 _ZN6svgren8Renderer16renderSvgElementERKN6svgdom9ContainerERKNS1_9StyleableERKNS1_9ViewBoxedERKNS1_13AspectRatioedERKNS1_6LengthESG_SGSG + 543 26 libsvgren.dylib 0x0000000118eccf77 _ZN6svgren8Renderer5visitERKN6svgdom10SvgElementE + 55 27 libsvgren.dylib 0x0000000118ed568b Render + 521

igagis commented 6 years ago

I made a simple test app: https://github.com/igagis/svgren/tree/master/tests/async Can you try running it in your environment, just to check if same issue can be reproduced with this app or it needs something more than taht?

igagis commented 6 years ago

You can just clone svgren git repo and run make test. Or, you can cd tests/async and then run make test to run only that test app.

Papirosnik commented 6 years ago

I am a little bit sceptical about your test. Can suppose that one attempt of rendering the same dom can mask a problem somehow.

Try to render all my 18 different svgs in parallel. Crash occurs during rendering 5th or 6th image or somewhere near.

igagis commented 6 years ago

Ok, I improved the test app, now it renders many files. And I could not reproduce the crash anymore on Linux. I tried your set of files, but since I cannot add those to the repo right now the test takes samples from samples test app. No crash reproduced on both sets of files.

Could you try that test app in your environment? I also will try it on Mac this evening when I have access to it.

Note, that it was clearly a bug that there was that call to cairo_debug_reset_static_data(), but since it is gone now, I see no crash. So, now I need a test app which reproduces the crash, so could you try it in your environment?

igagis commented 6 years ago

And by the way, test has passed on CI machine (Mac): https://travis-ci.org/igagis/svgren/jobs/371511824

Papirosnik commented 6 years ago

Alas, I cannot try your test app in my environment because it is too complicated. I wrap your library and all its dependcies into dynamic library in order to be used from managed C# code ) Well, will perform further investigations.

igagis commented 6 years ago

I see. One more thing to check is which version of cairo are you using? I think in some older version they had these multithreading issues, but I think in versions >=1.14.x it should be fixed.

Papirosnik commented 6 years ago

cairo_static.1.15.4.1 It might be possible that the source of multithreading issues is not cairo itself but its dependency pixman

igagis commented 6 years ago

So, as I understand you use C# threads, or tasks?

Papirosnik commented 6 years ago

Task.Run but I can suppose it doesn't matter

igagis commented 6 years ago

Do you use any kind of C# code obfuscation? I found this discussion https://www.pcreview.co.uk/threads/system-accessviolationexception-in-net-2-0-application.3179763/

igagis commented 6 years ago

Also, reading this thread: https://www.experts-exchange.com/questions/23069706/Calling-unmanaged-code-with-dllimport-from-a-multithreaded-app.html

there is a reply there saying this:

Did you know the .NET threads don't represent distinct Win32 threads. If you are using thread-local storage in your DLL, this may cause the issue.

Try using the Thread.BeginThreadAffinity method to force the managed thread to a particular Win32 thread id. You should do this on all threads that use the DLL so long as it is needed.

See http://msdn2.microsoft.com/en-us/library/system.threading.thread.beginthreadaffinity.aspx.

so, maybe you should try using threads instead of Task.Run and try this Thread.BeginThreadAffinity

Papirosnik commented 6 years ago

Thank you for the start point for investigation. Will do this a bit later.

igagis commented 6 years ago

I'm closing this issue for now, since multithreading was testd and works in Linux now. @Papirosnik If you find problems with your C# bindings to be a problem in svgren code, please open a new bug.