HakanL / WkHtmlToPdf-DotNet

C# .NET Core wrapper for wkhtmltopdf library that uses Webkit engine to convert HTML pages to PDF.
GNU Lesser General Public License v3.0
366 stars 66 forks source link

I didn't use dependency injection. Using static creation would cause the thread to get stuck and new requests to keep getting stuck #108

Closed xiaolei000129 closed 1 year ago

xiaolei000129 commented 1 year ago

image

HakanL commented 1 year ago

There's probably an issue in how you're using this in your code, you need to make sure you only have one instance of the SynchronizedConverter.

xiaolei000129 commented 1 year ago

I'm using a static constructor for a static class

xiaolei000129 commented 1 year ago

It's thread safe. Static constructors are executed only once the first time the class is accessed, and only by one thread in a multithreaded environment. Thus, static constructors initialize data thread-safe.

xiaolei000129 commented 1 year ago

image

image

I was wondering if this was the case, but during the testing process, I found that my expectations met

HakanL commented 1 year ago

Looks correct, but please share the code that is causing the problems.

xiaolei000129 commented 1 year ago

public static class HtmlToPdfHelper { private static IConverter _htmlToPdfConverter = null; static HtmlToPdfHelper() { _htmlToPdfConverter = new SynchronizedConverter(new PdfTools()); }

public static byte[] ConvertHtmlToPdf(HtmlToPdfBase request)
{
    if (request.HtmlContent.IsNullOrWhiteSpace() && request.URL.IsNullOrWhiteSpace())
        throw new BusinessException("html content can't be equal to empty");

    var myWriter = new StringWriter();
    HttpUtility.HtmlDecode(request.HtmlContent, myWriter);
    var doc = new HtmlToPdfDocument()
    {
        GlobalSettings =
        {
            ColorMode = request.ColorMode,
            Orientation = request.IsPortrait ? Orientation.Portrait : Orientation.Landscape,
            PaperSize = request.PaperSize,
            Margins= request.Margins,
        },
        Objects =
        {
                new ObjectSettings()
                {
                    PagesCount = request.IsShowPagesCount,
                    HtmlContent = myWriter.ToString(),
                    ProduceForms=request.IsGenerateForms,
                    WebSettings = {
                        DefaultEncoding = request.DefaultEncoding,
                        LoadImages = request.IsLoadImage
                    },
                    HeaderSettings = {
                        FontSize = request.FontSize,
                        Spacing = request.Spacing
                    },
                }
        }
    };
    return _htmlToPdfConverter.Convert(doc);
}

}

xiaolei000129 commented 1 year ago

The code above is my code

HakanL commented 1 year ago

BTW what do you mean with thread getting stuck? So the PDF generation is single-threaded, so it will only allow one PDF to be generated at a time, when it's finished the next will be processed and so on. It doesn't matter how many threads you'll throw at it, it won't run faster.

xiaolei000129 commented 1 year ago

The phenomenon is that all timeout freezes as long as the method is called

HakanL commented 1 year ago

But the first one will eventually complete or? How large are is the HTML content?

xiaolei000129 commented 1 year ago

The problem is occasional

xiaolei000129 commented 1 year ago

html size is uncertain

xiaolei000129 commented 1 year ago

It shouldn't be very big

HakanL commented 1 year ago

I would recommend that you add some logging to figure out what is going on, from a quick glance at your code it looks correct. My guess is that you're running a large converter job, and then you're getting timeouts on other requests because the first request is still processing.

xiaolei000129 commented 1 year ago

Ok, thank you. I'll keep an eye on it

xiaolei000129 commented 1 year ago

I know what the problem is Because I implemented an htmlto image myself with libwkhtmltox There may be multiple threads accessing the same libwkhtmltox.dll The current solution is to copy a set of libwkhtmltox.dll files for htmlto image

xiaolei000129 commented 1 year ago

Whether you need to support html to image

HakanL commented 1 year ago

Yeah, there can only be one instance (per appdomain). Or change your image method to use the same singleton.

xiaolei000129 commented 1 year ago

Will you provide html to image implementation?

xiaolei000129 commented 1 year ago

Otherwise I can only have two sets of instances

HakanL commented 1 year ago

I don't think it's in this wrapper currently, but if the native library supports it then I'm sure it's possible to add it, feel free to submit PR.

xiaolei000129 commented 1 year ago

Uh-huh