GDI Error when using Base64QRCode

ctsears commented 6 years ago

Type of issue

[x ] Bug
[ ] Question (e.g. about handling/usage)
[ ] Request for new feature/improvement

Expected Behavior

Using the Base64QRCode.GetGraphic() method should not cause a GDI error.

Current Behavior

Under certain conditions (low memory?) calling Base64QRCode.GetGraphic() results in a GDI error.

Possible Solution (optional)

We worked around this by using QRCode.GetGraphic() and doing the base64 conversion ourselves.

We think the problem stems from not properly disposing of the bitmap within Base64QRCode.GetGraphic()

This broke on our production server (but worked in development):

var subject = "Simple Email Test";
var body = "We used a pretty long string. That may be a factor? It was roughly 200 characters long.";
var mailPayloadGenerator = new QRCoder.PayloadGenerator.Mail("", subject , body);
var qrCodeData = new QRCodeGenerator().CreateQrCode(mailPayloadGenerator.ToString(), QRCodeGenerator.ECCLevel.Q);
var qrCodeString = new Base64QRCode(qrCodeData).GetGraphic(3);

This fixed it:

string qrCodeString = null;
var subject = "Simple Email Test";
var body = "We used a pretty long string. That may be a factor? It was roughly 200 characters long.";
var mailPayloadGenerator = new QRCoder.PayloadGenerator.Mail("", subject , body);
var qrCodeData = new QRCodeGenerator().CreateQrCode(mailPayloadGenerator.ToString(), QRCodeGenerator.ECCLevel.Q);
using (var bitmap = new QRCode(qrCodeData).GetGraphic(3))
using (var stream = new System.IO.MemoryStream())
{
    bitmap.Save(stream, ImageFormat.Png);
    var imageBytes = stream.ToArray();
    qrCodeString = Convert.ToBase64String(stream.ToArray());
}

Steps to Reproduce (for bugs)

This is the toughie. It broke on some machines and not others. Most likely, you'd need to test while the system is starved for RAM. The only thing noteworthy beyond that is we are using rather long text (~200 characters).

Your Environment

Version used: 1.3.2
Compiled from source or NuGet package?: Yes
Payload/Encoded text: ~200 characters of text for an email
Used payload generator: Mail
Used ECC-level: Q
Used renderer class: None (we send the base64 string to a browser)
Environment: .NET 4.6

codebude commented 6 years ago

Hi @ctsears ,

many thanks for this bug report. (It does not often happen that the report is that detailed.) I tried to reproduce your problem, but couldn't produce a GDI error.

I tried to create 10000 Base64QRCodes in a loop, but the Garbage Collector managed to keep the memory usage at a constant low level. After that I tried to create 10000 BAse64QRCodes in a Parallel.For-loop. This ended in a OutOfMemory-exception - but this isn't a bug. If no memory isn't available, the best code can't do anything...

Nevertheless I see an improvement in you "fixing solution". Using a using clause for Bitmap and MemoryStream seems to be a good idea. So I implemented it in this commit: https://github.com/codebude/QRCoder/commit/41543039c8884e896fb40e4f011f08353794d465

I hope this fixes your problem. Since I can't reproduce the bug, I close this issue with this comment/commit. Nevertheless - feel free to comment or give feedback if this solved your issue or if you know a way to re-produce the error.

codebude commented 6 years ago

@ctsears 30 minutes ago I released version 1.3.3 on Nuget. Your proposed changes are part of this release. So feel free to use the official Nuget package again.

codebude / QRCoder