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

Fix merging race conditions #570

Merged
merged 1 commit into from
Jul 17, 2017

Commits on Jul 17, 2017

  1. Fix merging race conditions

    Ensure we cache and synchronize appropriately so we don't merge more than
    once or do unsafe reads of the resultset. This has the added benefit of
    reducing the runtime of at_exit hooks by over 66%, since we now do less
    than 1/3 the work.
    
    See https://gist.github.com/jenseng/62465f674f8c02de09ef776f23d4dca4
    for simple repro script.
    
    Basically there are 3 main problems when using merging:
    
    1. `SimpleCov::ResultMerger.stored_data` doesn't synchronize its read,
       so it can see an empty file if another process is writing it.
    2. `SimpleCov::ResultMerger.resultset` calls `.stored_data` twice and
       never caches the result.
    3. `SimpleCov.result` doesn't cache the `@result`, so the default `at_exit`
       behavior causes it to store and merge 3 times. Due to 1. and 2.,
       this is extra bad
    
    This can cause the formatter to miss out on coverage data and/or get the
    wrong values for covered percentages.
    
    Furthermore, if you use OJ, `JSON.parse("") -> nil`, which means
    `.resultset` can be nil, so this race condition causes exceptions as
    seen in simplecov-ruby#406. In addition to fixing the race condition, also add ` || {}`
    to make  `.stored_data` a bit more robust and protect against an empty
    .resultset.json.
    jenseng committed Jul 17, 2017
    Configuration menu
    Copy the full SHA
    6f12dd6 View commit details
    Browse the repository at this point in the history