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

Improved re-entrancy Scope Lock for Session Value Layout Render #850

Merged
merged 18 commits into from
Aug 12, 2022

Conversation

bakgerman
Copy link
Contributor

@bakgerman bakgerman commented Aug 10, 2022

Also fix spelling errors and remove unused using statements. Resolves #847

…ling errors and remove unused using statements.
@codecov-commenter
Copy link

codecov-commenter commented Aug 10, 2022

Codecov Report

Merging #850 (7a52795) into master (499850e) will decrease coverage by 0%.
The diff coverage is 47%.

@@          Coverage Diff          @@
##           master   #850   +/-   ##
=====================================
- Coverage      69%    69%   -0%     
=====================================
  Files          63     65    +2     
  Lines        1157   1214   +57     
  Branches      286    309   +23     
=====================================
+ Hits          804    843   +39     
+ Misses        229    226    -3     
- Partials      124    145   +21     
Impacted Files Coverage Δ
...ayoutRenderers/AspNetSessionValueLayoutRenderer.cs 54% <36%> (-23%) ⬇️
src/Shared/Internal/ReEntrantScopeLock.cs 54% <54%> (ø)
...utRenderers/AspNetRequestDurationLayoutRenderer.cs 58% <0%> (-7%) ⬇️
...outRenderers/AspNetLayoutMultiValueRendererBase.cs 86% <0%> (-1%) ⬇️
src/NLog.Web/Internal/HostEnvironment.cs 0% <0%> (ø)
...Renderers/AspNetRequestPostedBodyLayoutRenderer.cs 86% <0%> (+1%) ⬆️
...LayoutRenderers/AspNetAppBasePathLayoutRenderer.cs 53% <0%> (+53%) ⬆️
...d/LayoutRenderers/IISInstanceNameLayoutRenderer.cs 67% <0%> (+67%) ⬆️
...LayoutRenderers/AspNetWebRootPathLayoutRenderer.cs 67% <0%> (+67%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@snakefoot
Copy link
Contributor

Could the Re-Entrant logic be put in an IDiposable-struct, that implements IDisposable. Where struct-constructor setup the blocker, and the dispose-method removes the blocker.

Then the IDiposable-struct can used in a using-statement to protect the lookup.

@bakgerman
Copy link
Contributor Author

Could the Re-Entrant logic be put in an IDiposable-struct, that implements IDisposable. Where struct-constructor setup the blocker, and the dispose-method removes the blocker.

Then the IDiposable-struct can used in a using-statement to protect the lookup.

That sounds like a great idea. We will do that.

@pull-request-size pull-request-size bot added size/L and removed size/M labels Aug 11, 2022
@snakefoot
Copy link
Contributor

snakefoot commented Aug 12, 2022 via email

@bakgerman
Copy link
Contributor Author

changed as requested. I did not know that about struct, so I learned something new.

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

79.4% 79.4% Coverage
0.0% 0.0% Duplication

@snakefoot
Copy link
Contributor

@bakgerman I think it looks great now. Do you have any more changes that you want to make ?

@snakefoot snakefoot added this to the 5.1.1 milestone Aug 12, 2022
@snakefoot snakefoot added the bug label Aug 12, 2022
@bakgerman
Copy link
Contributor Author

@bakgerman I think it looks great now. Do you have any more changes that you want to make ?

No, I'm done with this pull if you agree

@snakefoot snakefoot merged commit 23ffb7d into NLog:master Aug 12, 2022
@snakefoot snakefoot changed the title Re-Entrancy management for Session Value Layout Render. Re-Entrancy Scope Lock for Session Value Layout Render Aug 12, 2022
@snakefoot
Copy link
Contributor

Once again thank you for being patient with all my nit-picking.

@bakgerman bakgerman deleted the sessionvalue branch August 12, 2022 16:11
@snakefoot snakefoot changed the title Re-Entrancy Scope Lock for Session Value Layout Render Improved re-entrancy Scope Lock for Session Value Layout Render Aug 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AspNetSessionValueLayoutRenderer should be threadsafe
3 participants