Skip to content

fix: Base64QRCode chama base SetQRCodeData e dispoem o renderizador interno #90

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.

Sign up for GitHub

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

Merged
afonsoft merged 1 commit into from
Jul 12, 2026
Merged

fix: Base64QRCode chama base SetQRCodeData e dispoem o renderizador interno #90

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions QRCoder.Core.Tests/Renderers/Base64QRCodeTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,33 @@ public void can_instantiate_parameterless()
var b64Qr = new Base64QRCode();
b64Qr.ShouldNotBeNull();
}

[Fact]
public void can_set_data_and_dispose()
{
// Arrange
using (var gen = new QRCodeGenerator())
using (var data = gen.CreateQrCode("SetQRCodeData test", QRCodeGenerator.ECCLevel.M))
using (var b64Qr = new Base64QRCode())
{
// Act
b64Qr.SetQRCodeData(data);
var base64 = b64Qr.GetGraphic(10);

// Assert
base64.ShouldNotBeNullOrEmpty();
Convert.FromBase64String(base64).Length.ShouldBeGreaterThan(0);
}
}

[Fact]
public void parameterless_dispose_does_not_throw()
{
// Arrange
var b64Qr = new Base64QRCode();

// Act & Assert
Should.NotThrow(() => b64Qr.Dispose());
}
}
}
16 changes: 16 additions & 0 deletions QRCoder.Core/Renderers/Base64QRCode.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,25 @@ public Base64QRCode(QRCodeData data) : base(data)
/// <param name="data">The data.</param>
public override void SetQRCodeData(QRCodeData data)
{
base.SetQRCodeData(data);
this.qr.SetQRCodeData(data);
Comment on lines +92 to 93

@devin-ai-integration devin-ai-integration Bot Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: The SetQRCodeData fix corrects a pre-existing bug where base class data was never set

Before this PR, SetQRCodeData in Base64QRCode only called this.qr.SetQRCodeData(data) without calling base.SetQRCodeData(data). This meant that this.QrCodeData (the inherited property from AbstractQRCode) remained null when using the parameterless constructor + SetQRCodeData flow. While this didn't cause runtime issues because Base64QRCode delegates all rendering to the inner qr field (which did have the data), it violated the contract of AbstractQRCode and could confuse any code that inspects QrCodeData on the base class. The addition of base.SetQRCodeData(data) at QRCoder.Core/Renderers/Base64QRCode.cs:92 is a correct fix.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

}

/// <summary>
/// Releases the unmanaged resources and optionally releases the managed resources.
/// </summary>
/// <param name="disposing">true to release both managed and unmanaged resources; false to release only unmanaged resources.</param>
protected override void Dispose(bool disposing)
{
if (disposing)
{
this.qr?.Dispose();
this.QrCodeData = null;
}

base.Dispose(disposing);
}
Comment on lines +100 to +109

@devin-ai-integration devin-ai-integration Bot Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Shared QR code data is disposed twice during cleanup, once by the inner renderer and again by the base class

The shared data object is disposed by the inner renderer (this.qr?.Dispose() at QRCoder.Core/Renderers/Base64QRCode.cs:104) and then disposed again by the base class call, so the same object undergoes disposal twice per cleanup cycle.

Impact: Currently benign because the data's dispose only nulls fields, but will cause errors if that disposal ever performs non-idempotent cleanup.

Double-dispose mechanism through shared QRCodeData reference

When Base64QRCode is constructed via Base64QRCode(QRCodeData data) at line 81-84, both this.QrCodeData (set by base(data)) and this.qr.QrCodeData (set by new QRCode(data)) point to the same QRCodeData instance.

Similarly, when SetQRCodeData(data) is called (lines 90-94), both base.SetQRCodeData(data) and this.qr.SetQRCodeData(data) set the same reference.

During Dispose(true) (lines 100-108):

  1. this.qr?.Dispose() calls AbstractQRCode.Dispose(true) on the inner QRCode, which calls qr.QrCodeData?.Dispose() and sets qr.QrCodeData = null (QRCoder.Core/Abstractions/AbstractQRCode.cs:63-65)
  2. base.Dispose(disposing) then calls this.QrCodeData?.Dispose() on the SAME object (since this.QrCodeData still holds the reference) and sets this.QrCodeData = null

QRCodeData.Dispose() (QRCoder.Core/Models/QRCodeData.cs:214-221) has no _disposed guard, so its body executes twice.

Fix: Either don't call base.Dispose(disposing) (but then this.QrCodeData won't be nulled), or null out this.QrCodeData without disposing it (since qr already disposed it), or add a disposed guard.

Prompt for agents 
In Base64QRCode.Dispose(bool disposing), the inner `this.qr` and the base class both hold a reference to the same QRCodeData instance. When qr.Dispose() is called, it disposes the shared QRCodeData. Then base.Dispose(disposing) disposes it again.

To fix this, you should avoid the double-dispose. Options:
1. Set `this.QrCodeData = null` before calling `base.Dispose(disposing)` so the base class skips the dispose (since it checks `?.Dispose()`). But this accesses a protected field directly.
2. Override Dispose to only dispose qr and then null out QrCodeData without calling its Dispose again. For example:
   - Dispose qr (which disposes the shared QRCodeData)
   - Set this.QrCodeData = null (to prevent base from disposing it again)
   - Call base.Dispose(disposing) (which will see QrCodeData is null and skip it)
3. Add a _disposed flag to prevent re-entrance.

Option 2 is simplest: insert `this.QrCodeData = null;` between `this.qr?.Dispose();` and `base.Dispose(disposing);` in the Dispose override at QRCoder.Core/Renderers/Base64QRCode.cs:100-108.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@devin-ai-integration devin-ai-integration Bot Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrigido. O Dispose agora nulifica this.QrCodeData depois de dispor o qr interno, evitando o duplo Dispose no mesmo objeto. O base.Dispose segue chamado para manter a cadeia de descarte e garantir o idempotence do padrão.

Comment on lines +100 to +109

@devin-ai-integration devin-ai-integration Bot Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: The Dispose override fixes a resource leak where the inner QRCode was never cleaned up

Before this PR, Base64QRCode had no Dispose(bool) override, so when disposed, only base.Dispose(true) ran — which disposed this.QrCodeData but never disposed the inner this.qr field. The QRCode instance (which is itself an AbstractQRCode with its own QrCodeData reference) was leaked. The new override at QRCoder.Core/Renderers/Base64QRCode.cs:100-108 correctly addresses this leak, though it introduces a double-dispose of the shared QRCodeData (reported separately as a bug).

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


/// <summary>
/// Returns the graphic representation of the QR code.
/// </summary>
Expand Down
Loading