Skip to content
This repository has been archived by the owner on Aug 30, 2023. It is now read-only.

clone method - race condition free #226

Merged
merged 9 commits into from Jan 19, 2020
Merged

clone method - race condition free #226

merged 9 commits into from Jan 19, 2020

Conversation

marandaneto
Copy link
Contributor

馃摙 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

馃摐 Description

clone race condition free.

馃挕 Motivation and Context

because the clone methods might give some race conditions.

馃挌 How did you test it?

just ran the tests.

馃摑 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • All tests passing

馃敭 Next steps

@codecov-io
Copy link

codecov-io commented Jan 14, 2020

Codecov Report

Merging #226 into master will decrease coverage by 0.37%.
The diff coverage is 88.88%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #226      +/-   ##
============================================
- Coverage     58.27%   57.89%   -0.38%     
+ Complexity      573      567       -6     
============================================
  Files            72       72              
  Lines          2701     2691      -10     
  Branches        231      230       -1     
============================================
- Hits           1574     1558      -16     
- Misses         1016     1018       +2     
- Partials        111      115       +4
Impacted Files Coverage 螖 Complexity 螖
sentry-core/src/main/java/io/sentry/core/Hub.java 68.63% <80%> (-1.55%) 52 <0> (-3)
...entry-core/src/main/java/io/sentry/core/Scope.java 82.1% <90.9%> (-1.42%) 29 <5> (-1)
...rc/main/java/io/sentry/core/CircularFifoQueue.java 34.95% <0%> (-2.44%) 17% <0%> (-1%)
...try/core/transport/RetryingThreadPoolExecutor.java 53.76% <0%> (-1.08%) 13% <0%> (-1%)

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 f3697b4...82156bf. Read the comment docs.

Copy link
Contributor Author

@marandaneto marandaneto left a comment

Choose a reason for hiding this comment

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

@metlos hey :) would you have 2 cents to contribute here?

we aim to be fully thread-safe and guess our clone methods had race conditions, right now I'm using "reference assignment" as it's supposed to be atomic.

I've tried to implement a deep copy, but not sure if I'm happy with it yet.

Also I've changed ArrayList to the Concurrent ones.

Unfortunately, we cannot be fully immutable, so that makes a bit harder.

Thanks :)

@marandaneto marandaneto changed the title clone race condition free clone method - race condition free Jan 18, 2020
@marandaneto marandaneto marked this pull request as ready for review January 18, 2020 10:54
@marandaneto marandaneto merged commit d2655be into master Jan 19, 2020
@marandaneto marandaneto deleted the ref/clone branch January 19, 2020 10:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants