Merge pull request #321 from WhiteBlackGoose/async-fix

Rewriting Async API in ImageExport

src/Plotly.NET.ImageExport/AsyncHelper.fs

@@ -0,0 +1,77 @@
+module Plotly.NET.ImageExport.AsyncHelper
+
+open System.Threading
+open System.Threading.Tasks
+
+(*
+
+This is a workaround to avoid deadlocks
+
+https://medium.com/rubrikkgroup/understanding-async-avoiding-deadlocks-e41f8f2c6f5d
+ 
+TL;DR in many cases, for example, GUI apps, SynchronizationContext
+is overriden to *post* the executing code on the initial (UI) thread. For example,
+consider this code
+ 
+public async Task OnClick1()
+{
+    var chart = ...;
+    var base64 = ImageExport.toBase64PNGStringAsync()(chart).Result;
+    myButton.Text = base64;
+}
+
+Here we have an async method. Normally you should use await and not use .Result, but
+assume for some reason the sync version is used. What happens under the hood is,
+
+public async Task OnClick1()
+{
+    var chart = ...;
+    var task = ImageExport.toBase64PNGStringAsync()(chart);
+    task.ContinueWith(() =>
+        UIThread.Schedule(() =>
+            myButton.Text = Result;
+        )
+    );
+    task.Wait();
+}
+
+(this is pseudo-code)
+
+So basically, we set the task to wait until it finishes. However, part of it being
+finished is to actually execute the code with button.Text = .... The waiting happens
+on the UI thread, exactly on the same thread as where we're waiting for it to do
+another job!
+
+That's not the only place we potentially deadlock by using fake synchronous functions.
+The reason why it happens, is because frameworks (or actually anyone) override
+SynchronizationContext. In GUI and game development it's very useful to keep UI logic
+on one thread. But our rendering does not ever callback to it, we're independent of
+where the logic actually happens.
+
+That's why what we do is we set the synchronization context to null, do the job, and
+then restore it. It is a workaround, because it doesn't have to work everywhere and
+independently. But it will work for most cases.
+
+When will it also break? For example, if we decide to take in some callback as a para-
+meter, which potentially accesses the UI thread (or whatever). In Unity, for instance,
+you can only access Unity API from the main thread. So our fake synchronous function
+will crash in the end, because due to the overriden (by us) sync context, the callback
+will be executed in some random thread (as opposed to being posted back to the UI one).
+
+However, our solution should work in most cases.
+
+Credit to [@DaZombieKiller](https://github.com/DaZombieKiller) for helping.
+ 
+*)
+
+let runSync job input =
+    let current = SynchronizationContext.Current
+    SynchronizationContext.SetSynchronizationContext null
+    try
+        job input
+    finally
+        SynchronizationContext.SetSynchronizationContext current
+
+let taskSync (task : Task<'a>) = task |> runSync (fun t -> t.Result)
+
+let taskSyncUnit (task : Task) = task |> runSync (fun t -> t.Wait())

src/Plotly.NET.ImageExport/ChartExtensions.fs

@@ -56,7 +56,7 @@ module ChartExtensions =
             fun (gChart: GenericChart) ->
                 gChart
                 |> Chart.toBase64JPGStringAsync (?EngineType = EngineType, ?Width = Width, ?Height = Height)
-                |> Async.RunSynchronously
+                |> AsyncHelper.taskSync
 
         /// <summary>
         /// Returns an async function that saves a GenericChart as JPG image
@@ -97,7 +97,7 @@ module ChartExtensions =
             fun (gChart: GenericChart) ->
                 gChart
                 |> Chart.saveJPGAsync (path, ?EngineType = EngineType, ?Width = Width, ?Height = Height)
-                |> Async.RunSynchronously
+                |> AsyncHelper.taskSync
 
         /// <summary>
         /// Returns an async function that converts a GenericChart to a base64 encoded PNG string
@@ -134,7 +134,7 @@ module ChartExtensions =
             fun (gChart: GenericChart) ->
                 gChart
                 |> Chart.toBase64PNGStringAsync (?EngineType = EngineType, ?Width = Width, ?Height = Height)
-                |> Async.RunSynchronously
+                |> AsyncHelper.taskSync
 
         /// <summary>
         /// Returns an async function that saves a GenericChart as PNG image
@@ -175,7 +175,7 @@ module ChartExtensions =
             fun (gChart: GenericChart) ->
                 gChart
                 |> Chart.savePNGAsync (path, ?EngineType = EngineType, ?Width = Width, ?Height = Height)
-                |> Async.RunSynchronously
+                |> AsyncHelper.taskSync
 
         /// <summary>
         /// Returns an async function that converts a GenericChart to a SVG string
@@ -211,7 +211,7 @@ module ChartExtensions =
             fun (gChart: GenericChart) ->
                 gChart
                 |> Chart.toSVGStringAsync (?EngineType = EngineType, ?Width = Width, ?Height = Height)
-                |> Async.RunSynchronously
+                |> AsyncHelper.taskSync
 
         /// <summary>
         /// Returns an async function that saves a GenericChart as SVG image
@@ -251,4 +251,4 @@ module ChartExtensions =
             fun (gChart: GenericChart) ->
                 gChart
                 |> Chart.saveSVGAsync (path, ?EngineType = EngineType, ?Width = Width, ?Height = Height)
-                |> Async.RunSynchronously
+                |> AsyncHelper.taskSync

src/Plotly.NET.ImageExport/IGenericChartRenderer.fs

@@ -1,5 +1,6 @@
 namespace Plotly.NET.ImageExport
 
+open System.Threading.Tasks
 open Plotly.NET
 
 /// <summary>
@@ -8,31 +9,31 @@ open Plotly.NET
 type IGenericChartRenderer =
 
     ///<summary>Async function that returns a base64 encoded string representing the input chart as JPG file with the given width and height</summary>
-    abstract member RenderJPGAsync: int * int * GenericChart.GenericChart -> Async<string>
+    abstract member RenderJPGAsync: int * int * GenericChart.GenericChart -> Task<string>
     ///<summary>Function that returns a base64 encoded string representing the input chart as JPG file with the given width and height</summary>
     abstract member RenderJPG: int * int * GenericChart.GenericChart -> string
 
     ///<summary>Async function that saves the input chart as JPG file with the given width and height at the given path</summary>
-    abstract member SaveJPGAsync: string * int * int * GenericChart.GenericChart -> Async<unit>
+    abstract member SaveJPGAsync: string * int * int * GenericChart.GenericChart -> Task<unit>
     ///<summary>Function that saves the input chart as JPG file with the given width and height at the given path</summary>
     abstract member SaveJPG: string * int * int * GenericChart.GenericChart -> unit
 
     ///<summary>Async function that returns a base64 encoded string representing the input chart as PNG file with the given width and height</summary>
-    abstract member RenderPNGAsync: int * int * GenericChart.GenericChart -> Async<string>
+    abstract member RenderPNGAsync: int * int * GenericChart.GenericChart -> Task<string>
     ///<summary>Function that returns a base64 encoded string representing the input chart as PNG file with the given width and height</summary>
     abstract member RenderPNG: int * int * GenericChart.GenericChart -> string
 
     ///<summary>Async function that saves the input chart as PNG file with the given width and height at the given path</summary>
-    abstract member SavePNGAsync: string * int * int * GenericChart.GenericChart -> Async<unit>
+    abstract member SavePNGAsync: string * int * int * GenericChart.GenericChart -> Task<unit>
     ///<summary>Function that saves the input chart as PNG file with the given width and height at the given path</summary>
     abstract member SavePNG: string * int * int * GenericChart.GenericChart -> unit
 
     ///<summary>Async function that returns a string representing the input chart as SVG file with the given width and height</summary>
-    abstract member RenderSVGAsync: int * int * GenericChart.GenericChart -> Async<string>
+    abstract member RenderSVGAsync: int * int * GenericChart.GenericChart -> Task<string>
     ///<summary>Function that returns string representing the input chart as SVG file with the given width and height</summary>
     abstract member RenderSVG: int * int * GenericChart.GenericChart -> string
 
     ///<summary>Async function that saves the input chart as SVG file with the given width and height at the given path</summary>
-    abstract member SaveSVGAsync: string * int * int * GenericChart.GenericChart -> Async<unit>
+    abstract member SaveSVGAsync: string * int * int * GenericChart.GenericChart -> Task<unit>
     ///<summary>Function that saves the input chart as SVG file with the given width and height at the given path</summary>
     abstract member SaveSVG: string * int * int * GenericChart.GenericChart -> unit

src/Plotly.NET.ImageExport/Plotly.NET.ImageExport.fsproj

@@ -25,6 +25,7 @@
   <ItemGroup>
     <None Include="..\..\docs\img\logo.png" Pack="true" PackagePath="\" />
     <None Include="RELEASE_NOTES.md" />
+    <Compile Include="AsyncHelper.fs" />
     <Compile Include="IGenericChartRenderer.fs" />
     <Compile Include="PuppeteerSharpRenderer.fs" />
     <Compile Include="ExportEngine.fs" />

src/Plotly.NET.ImageExport/PuppeteerSharpRenderer.fs

@@ -1,5 +1,7 @@
 namespace Plotly.NET.ImageExport
 
+open System.Threading
+open System.Threading.Tasks
 open Plotly.NET
 open PuppeteerSharp
 
@@ -20,7 +22,7 @@ module PuppeteerSharpRendererOptions =
 
 
 type PuppeteerSharpRenderer() =
-
+    
     /// adapted from the original C# implementation by @ilyalatt : https://github.com/ilyalatt/Plotly.NET.PuppeteerRenderer
     ///
     /// creates a full screen html site for the given chart
@@ -61,7 +63,7 @@ type PuppeteerSharpRenderer() =
     ///
     /// attempts to render a chart as static image of the given format with the given dimensions from the given html string
     let tryRenderAsync (browser: Browser) (width: int) (height: int) (format: StyleParam.ImageFormat) (html: string) =
-        async {
+        task {
             let! page = browser.NewPageAsync() |> Async.AwaitTask
 
             try
@@ -71,41 +73,33 @@ type PuppeteerSharpRenderer() =
                 return imgStr
 
             finally
-                page.CloseAsync() |> Async.AwaitTask |> Async.RunSynchronously
+                page.CloseAsync() |> AsyncHelper.taskSyncUnit
         }
 
-    /// attempts to render a chart as static image of the given format with the given dimensions from the given html string
-    let tryRender (browser: Browser) (width: int) (height: int) (format: StyleParam.ImageFormat) (html: string) =
-        tryRenderAsync browser width height format html |> Async.RunSynchronously
-
     /// Initalizes headless browser
     let fetchAndLaunchBrowserAsync () =
-        async {
+        task {
             match PuppeteerSharpRendererOptions.localBrowserExecutablePath with
             | None ->
                 use browserFetcher = new BrowserFetcher()
 
-                let! revision = browserFetcher.DownloadAsync() |> Async.AwaitTask
+                let! revision = browserFetcher.DownloadAsync()
 
                 let launchOptions =
                     PuppeteerSharpRendererOptions.launchOptions
 
                 launchOptions.ExecutablePath <- revision.ExecutablePath
 
-                return! Puppeteer.LaunchAsync(launchOptions) |> Async.AwaitTask
+                return! Puppeteer.LaunchAsync(launchOptions)
             | Some p ->
                 let launchOptions =
                     PuppeteerSharpRendererOptions.launchOptions
 
                 launchOptions.ExecutablePath <- p
 
-                return! Puppeteer.LaunchAsync(launchOptions) |> Async.AwaitTask
+                return! Puppeteer.LaunchAsync(launchOptions)
         }
 
-    /// Initalizes headless browser
-    let fetchAndLaunchBrowser () =
-        fetchAndLaunchBrowserAsync () |> Async.RunSynchronously
-
     /// skips the data type part of the given URI
     let skipDataTypeString (base64: string) =
         let imgBase64StartIdx =
@@ -120,7 +114,7 @@ type PuppeteerSharpRenderer() =
     interface IGenericChartRenderer with
 
         member this.RenderJPGAsync(width: int, height: int, gChart: GenericChart.GenericChart) =
-            async {
+            task {
                 use! browser = fetchAndLaunchBrowserAsync ()
 
                 return! tryRenderAsync browser width height StyleParam.ImageFormat.JPEG (gChart |> toFullScreenHtml)
@@ -129,10 +123,10 @@ type PuppeteerSharpRenderer() =
         member this.RenderJPG(width: int, height: int, gChart: GenericChart.GenericChart) =
             (this :> IGenericChartRenderer)
                 .RenderJPGAsync(width, height, gChart)
-            |> Async.RunSynchronously
+            |> AsyncHelper.taskSync
 
         member this.SaveJPGAsync(path: string, width: int, height: int, gChart: GenericChart.GenericChart) =
-            async {
+            task {
                 let! rendered =
                     (this :> IGenericChartRenderer)
                         .RenderJPGAsync(width, height, gChart)
@@ -143,10 +137,10 @@ type PuppeteerSharpRenderer() =
         member this.SaveJPG(path: string, width: int, height: int, gChart: GenericChart.GenericChart) =
             (this :> IGenericChartRenderer)
                 .SaveJPGAsync(path, width, height, gChart)
-            |> Async.RunSynchronously
+            |> AsyncHelper.taskSync
 
         member this.RenderPNGAsync(width: int, height: int, gChart: GenericChart.GenericChart) =
-            async {
+            task {
                 use! browser = fetchAndLaunchBrowserAsync ()
 
                 return! tryRenderAsync browser width height StyleParam.ImageFormat.PNG (gChart |> toFullScreenHtml)
@@ -155,10 +149,10 @@ type PuppeteerSharpRenderer() =
         member this.RenderPNG(width: int, height: int, gChart: GenericChart.GenericChart) =
             (this :> IGenericChartRenderer)
                 .RenderPNGAsync(width, height, gChart)
-            |> Async.RunSynchronously
+            |> AsyncHelper.taskSync
 
         member this.SavePNGAsync(path: string, width: int, height: int, gChart: GenericChart.GenericChart) =
-            async {
+            task {
                 let! rendered =
                     (this :> IGenericChartRenderer)
                         .RenderPNGAsync(width, height, gChart)
@@ -169,10 +163,10 @@ type PuppeteerSharpRenderer() =
         member this.SavePNG(path: string, width: int, height: int, gChart: GenericChart.GenericChart) =
             (this :> IGenericChartRenderer)
                 .SavePNGAsync(path, width, height, gChart)
-            |> Async.RunSynchronously
+            |> AsyncHelper.taskSync
 
         member this.RenderSVGAsync(width: int, height: int, gChart: GenericChart.GenericChart) =
-            async {
+            task {
                 use! browser = fetchAndLaunchBrowserAsync ()
 
                 let! renderedString =
@@ -184,10 +178,10 @@ type PuppeteerSharpRenderer() =
         member this.RenderSVG(width: int, height: int, gChart: GenericChart.GenericChart) =
             (this :> IGenericChartRenderer)
                 .RenderSVGAsync(width, height, gChart)
-            |> Async.RunSynchronously
+            |> AsyncHelper.taskSync
 
         member this.SaveSVGAsync(path: string, width: int, height: int, gChart: GenericChart.GenericChart) =
-            async {
+            task {
                 let! rendered =
                     (this :> IGenericChartRenderer)
                         .RenderSVGAsync(width, height, gChart)
@@ -198,4 +192,4 @@ type PuppeteerSharpRenderer() =
         member this.SaveSVG(path: string, width: int, height: int, gChart: GenericChart.GenericChart) =
             (this :> IGenericChartRenderer)
                 .SaveSVGAsync(path, width, height, gChart)
-            |> Async.RunSynchronously
+            |> AsyncHelper.taskSync

tests/Plotly.NET.ImageExport.Tests/ImageExport.fs

@@ -29,7 +29,7 @@ let ``Image export tests`` =
             ptestAsync "Chart.toBase64JPGStringAsync" {
                 let testBase64JPG = readTestFilePlatformSpecific "TestBase64JPG.txt"
 
-                let! actual = (Chart.Point([1.,1.]) |> Chart.toBase64JPGStringAsync())
+                let! actual = (Chart.Point([1.,1.]) |> Chart.toBase64JPGStringAsync() |> Async.AwaitTask)
 
                 return 
                     Expect.equal
@@ -40,13 +40,22 @@ let ``Image export tests`` =
             ptestAsync "Chart.toBase64PNGStringAsync" {
                 let testBase64PNG = readTestFilePlatformSpecific "TestBase64PNG.txt"
 
-                let! actual = (Chart.Point([1.,1.]) |> Chart.toBase64PNGStringAsync())
+                let! actual = (Chart.Point([1.,1.]) |> Chart.toBase64PNGStringAsync() |> Async.AwaitTask)
 
                 return 
                     Expect.equal
                         actual
                         testBase64PNG
                         "Invalid base64 string for Chart.toBase64PNGStringAsync"
             }
+            testCase "Chart.toBase64JPGString terminates" <| fun () ->
+                let actual = Chart.Point([1.,1.]) |> Chart.toBase64JPGString()
+                Expect.isTrue (actual.Length > 100) ""
+            testCase "Chart.toBase64PNGString terminates" <| fun () ->
+                let actual = Chart.Point([1.,1.]) |> Chart.toBase64PNGString()
+                Expect.isTrue (actual.Length > 100) ""
+            testCase "Chart.toSVGString terminates" <| fun () ->
+                let actual = Chart.Point([1.,1.]) |> Chart.toSVGString()
+                Expect.isTrue (actual.Length > 100) ""
         ]
     )