Skip to content

Conversation

@JimBobSquarePants
Copy link
Member

@JimBobSquarePants JimBobSquarePants commented Jun 16, 2020

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following matches the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

Fix #1230 #1224

The two dictionaries used in Configuration and DefaultImageProcessorContext were not thread safe and would throw when accessed in a parallel context.

I've replaced them with ConcurrentDictionary instances which fixes the issue but I wonder whether we should be exposing the property as IDictionary<object, object> over the concrete type since we require thread safety.

@JimBobSquarePants JimBobSquarePants added this to the 1.0.0 milestone Jun 16, 2020
@JimBobSquarePants JimBobSquarePants requested a review from a team June 16, 2020 21:09
@codecov
Copy link

codecov bot commented Jun 16, 2020

Codecov Report

Merging #1234 into master will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1234      +/-   ##
==========================================
- Coverage   82.54%   82.54%   -0.01%     
==========================================
  Files         697      697              
  Lines       30512    30511       -1     
  Branches     3469     3469              
==========================================
- Hits        25187    25186       -1     
  Misses       4605     4605              
  Partials      720      720              
Flag Coverage Δ
#unittests 82.54% <100.00%> (-0.01%) ⬇️
Impacted Files Coverage Δ
src/ImageSharp/Configuration.cs 100.00% <100.00%> (ø)
src/ImageSharp/GraphicOptionsDefaultsExtensions.cs 100.00% <100.00%> (ø)
...Processing/DefaultImageProcessorContext{TPixel}.cs 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a538d4...e95c85a. Read the comment docs.

Copy link
Member

@antonfirsov antonfirsov left a comment

Choose a reason for hiding this comment

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

LGTM.

@tocsoft tocsoft merged commit 53da0e5 into master Jun 17, 2020
@tocsoft tocsoft deleted the js/fix-1230 branch June 17, 2020 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

context.BackgroundColor throws Object reference not set to an instance of an object.

3 participants