jonwagner / Insight.Database

Fast, lightweight .NET micro-ORM
Other
861 stars 145 forks source link

Implement OneToOne.GetHashCode() to fix a memory leak #506

Closed ryan-westenhover closed 7 months ago

ryan-westenhover commented 8 months ago

Description

This PR fixes a memory leak that was happening in Insight.Database.CodeGenerator.DbReaderDeserializer caused by GetHashCode not being implemented in the OneToOne mapping class.

There is an existing pre-calculated _hashCode field that looks appropriate for this class, but it was never exposed through the GetHashCode method. This led to an issue where the static ConcurrentDictionary _deserializers in DbReaderDeserializer that is caching the deserializers was not recognizing that a deserializer already exists for a particular reader when checking if it needs to add a new one, ultimately leading to new deserializers accumulating in that cache over time and never getting released.

Checklist

Please make sure your pull request fulfills the following requirements:

Type

This pull request includes what type of changes?

Breaking Changes

Does this pull request introduce any breaking changes?

Any other comment

(n/a)

jonwagner commented 8 months ago

Cool. I have time to do a build next week. I’ll merge in the outstanding prs

On Mar 26, 2024, at 12:12 PM, ryan-westenhover @.***> wrote:

 Description

This PR fixes a memory leak that was happening in Insight.Database.CodeGenerator.DbReaderDeserializer caused by GetHashCode not being implemented in the OneToOne mapping class.

There is an existing pre-calculated _hashCode field that looks appropriate for this class, but it was never exposed through the GetHashCode method. This led to an issue where the static ConcurrentDictionary _deserializers in DbReaderDeserializer that is caching the deserializers was not recognizing that a deserializer already exists for a particular reader when checking if it needs to add a new one, ultimately leading to new deserializers accumulating in that cache over time and never getting released.

Checklist

Please make sure your pull request fulfills the following requirements:

Type

This pull request includes what type of changes?

Breaking Changes

Does this pull request introduce any breaking changes?

Any other comment

(n/a)


You can view, comment on, or merge this pull request online at:

https://github.com/jonwagner/Insight.Database/pull/506

Commit Summary

File Changes

(2 fileshttps://github.com/jonwagner/Insight.Database/pull/506/files)

Patch Links:

— Reply to this email directly, view it on GitHubhttps://github.com/jonwagner/Insight.Database/pull/506, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAMTO5ADKOGJTZ6GVGIQXLLY2GM5JAVCNFSM6AAAAABFJHYQ4OVHI2DSMVQWIX3LMV43ASLTON2WKOZSGIYDQNRYGQ2TSNQ. You are receiving this because you are subscribed to this thread.Message ID: @.***>