[Html2PdfConverter] Html2PdfConverter concurrency issue

Forge Component
(68)
Published on 5 Feb (3 weeks ago) by Miguel 'Kelter' Antunes
68 votes
Published on 5 Feb (3 weeks ago) by Miguel 'Kelter' Antunes

Hi guys,


We recently had to customize 'HtmlConvertToPdf' extension due to a concurrency issue while generating the temporary file name.


We are currently using a BPT process to generate files and the random name generation based on timestamp was not robust enough since multiple processes can easily bump into each other.


So we applied the following code change on method generateRandomName():


Original: Random rnd = new Random((int)System.DateTime.Now.Ticks);

New: Random rnd = new Random(BitConverter.ToInt32(Guid.NewGuid().ToByteArray(), 0));


Hope you guys can address this issue on a future release of Html2PdfConverter by using our approach or any other that you find more suitable.


Kind regards,

Luís Biscaia 

Hi Luis,


That's a good suggestion and I'll definitely will take it into consideration on the next version.


Did you happen do measure for any performance impacts with that change?


Cheers,

Guilherme

Guilherme Pereira wrote:

Hi Luis,


That's a good suggestion and I'll definitely will take it into consideration on the next version.


Did you happen do measure for any performance impacts with that change?


Cheers,

Guilherme

Hi Guilherme,


Glad to now that you will take it into consideration. 


We did not measure performance, no impact was noticed in the overall BPT process execution so we felt no need to take specific measures on the components performance.


Kind regards,

Luis Biscaia

Guilherme Pereira wrote:

Hi Luis,


That's a good suggestion and I'll definitely will take it into consideration on the next version.


Did you happen do measure for any performance impacts with that change?


Cheers,

Guilherme

Hi Guilherme,

This was already implemented in version 1.1.7. Can you tell us why it's changed back to this datetime based input?

Original Post: https://www.outsystems.com/forums/discussion/22665/the-process-cannot-access-the-file-because-it-is-being-used-by-another-proce/

Version changelog of 1.1.7:

Version 1.1.7

Application Package

Published on 23 June 2017 by Guilherme Pereira

  • Updated to OutSystems Platform 10.0.X
  • Introduced a new mechanism of generation of temporary pages based on RNGCryptoServiceProvider to decrease the possibility of conflicts on concurrency scenarios (thanks to Peter van den Ochtend for the suggestion)
  • New Demo page

Peter Van Den Ochtend wrote:

Guilherme Pereira wrote:

Hi Luis,


That's a good suggestion and I'll definitely will take it into consideration on the next version.


Did you happen do measure for any performance impacts with that change?


Cheers,

Guilherme

Hi Guilherme,

This was already implemented in version 1.1.7. Can you tell us why it's changed back to this datetime based input?

Original Post: https://www.outsystems.com/forums/discussion/22665/the-process-cannot-access-the-file-because-it-is-being-used-by-another-proce/

Version changelog of 1.1.7:

Version 1.1.7

Application Package

Published on 23 June 2017 by Guilherme Pereira

  • Updated to OutSystems Platform 10.0.X
  • Introduced a new mechanism of generation of temporary pages based on RNGCryptoServiceProvider to decrease the possibility of conflicts on concurrency scenarios (thanks to Peter van den Ochtend for the suggestion)
  • New Demo page


Hi Peter,


Yes you are correct. 


Unfortunately during the upgrade to the latest component version we bumped again into the same problem. 


We had to recover your'e original GUID fix, let's hope this time the component Team can have a fix for this issue that allows us to stop working with a cloned version. :P


Thanks for recovering the first post  we did on this matter.


Kind regards,

Luis Biscaia

My team has, as well, come across this issue due to creating many PDFs really fast during a business process. By switching just to GUID generation and basically making that the temp name, we were able to solve the issue.

After looking at the code, yes, there was a GUID re-added, but apparently it wasn't tested because someone missed that new Guid() just creates a GUID of 0's.

The correct code to create a GUID with actual random numbers/letters is this: Guid.NewGuid()

If you want to convert to a string and remove the dashes, you can do this: Guid.NewGuid().ToString("N") instead of a .Replace(), to make the code more concise.

Honestly, I would get rid of the random generator method entirely. The chances of GUID collisions are astronomical and there is no need to create extra overhead with convoluted developer-generated solutions for random value generation. Just put the second line of code I provided inline with the "TEMP" concatenation, where the call to the random generator currently is located.

Numbers on collision can be found here:
https://blog.stephencleary.com/2010/11/few-words-on-guids.html