Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Converting in gofuncs failed #3

Closed
shaikemarc opened this issue Aug 15, 2019 · 15 comments
Closed

Converting in gofuncs failed #3

shaikemarc opened this issue Aug 15, 2019 · 15 comments

Comments

@shaikemarc
Copy link

I tried to spawn x gofuncs, each created a converter but failed on access violation. Is this library safe for a concurrency use?

Thanks

@adrg
Copy link
Owner

adrg commented Oct 12, 2019

@shaikemarc The package itself should be thread safe. However, wkhtmltox does not seem to support multithreading. Relevant links:

wkhtmltopdf/wkhtmltopdf#1711
https://forum.dlang.org/thread/qmnmirbwmrvwexaewmtw@forum.dlang.org

I will try to find a solution to this issue, as running multiple converters concurrently is a common use case.

@shaikemarc
Copy link
Author

@adrg thanks for that!

@leandrosilva
Copy link

@adrg
Copy link
Owner

adrg commented Oct 15, 2020

@leandrosilva @shaikemarc I got a bit of free time and I thought I should give this issue a go. I managed to get it to work by running the conversion on the main thread. I added a Web page to PDF conversion server example to showcase this.

Would you be able to try the example and let me know if it works for you as well?

@leandrosilva
Copy link

As per my tests, yes, it has worked. Including the case with "window status".

I've been playing with wkhtmltox in other languages for awhile now (C++/Rust/Python/C#) and faced this "main thread" kind of problem many times until finally understand what happens.

This thread covers this topic quit well:
https://forum.qt.io/topic/84485/qapplication-not-created-in-main-thread/9

Thanks for sharing your code.

@leandrosilva
Copy link

I added runtime.LockOSThread to the example I wrote using my original HTTP Handler (pdf.ConvertPostHandler) and it worked a charm.

https://github.com/leandrosilva/go-wkhtmltopdf/blob/master/examples/use-of-http-handler/main.go

Many thanks. I didn't know this method.

@adrg
Copy link
Owner

adrg commented Oct 16, 2020

@leandrosilva That's great, I'm glad it worked for you as well. That means this issue can be closed.

@adrg adrg closed this as completed Oct 16, 2020
@adrg
Copy link
Owner

adrg commented Oct 16, 2020

@leandrosilva I added a couple of improvement in PR #6, including JSON/YAML marshal support for the conversion options. That should make it easier to parse HTTP conversion requests. Also added a Configurable web page to PDF conversion server example.

@leandrosilva
Copy link

That's cool.
I've done sort of that with ConvertOptions.
But that code is only on my branch and not in yours.
I'll refactor that at some point and converge to the same master as of yours.
If you have time, take a look at what I did with handler.go and use-of-http-handler example, perhaps you would like to include these to your master branch or something (with some refactoring, now we have ConverterOpts).
Cheers.

@leandrosilva
Copy link

BTW, not sure whether it happens to you but I got "could not set converter option outlineDepth to 4" everytime. It only works if I set it to 0. Anything different of that fails.

@leandrosilva
Copy link

I just did that: #7
Cheers

@adrg
Copy link
Owner

adrg commented Oct 18, 2020

@leandrosilva Hm, I don't have the outlineDepth issue. What version of wkhtmltopdf are you using, and on what OS? Perhaps the default should be 0 if different versions behave differently. I'm using version 0.12.6, downloaded from here.

@leandrosilva
Copy link

@adrg I'm using wkhtmltopdf 0.12.4 (with patched qt) on Windows, due to my job. We might default it to 0, to work around that issue, but I know the documentation for 0.12.6 says its default is 4. I guess we can just leave it as it is.

@adrg
Copy link
Owner

adrg commented Oct 18, 2020

From what I can tell, the default has always been 4. The problem with 0.12.4 is when you set that property, right? If you don't set it at all, everything works ok? If that's the case, if we have the default be 0 in the package, that would result in the outlineDepth property not being set, and it would default to 4 in versions that support this property.

@adrg
Copy link
Owner

adrg commented Oct 18, 2020

@leandrosilva I created a Discord channel so that issues like this one can be discussed in depth, or where users of the package can get support, etc. I added an invite link in README.md. It would be great to have you there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants