Skip to content
This repository was archived by the owner on Nov 30, 2024. It is now read-only.

Lock example_status_persistence_file to prevent race conditions #2029

Merged
merged 1 commit into from
Jul 16, 2015

Conversation

Empact
Copy link
Contributor

@Empact Empact commented Jul 15, 2015

This introduces file locking to ExampleStatusPersister#persist to
avoid a race condition where parallel or unrelated spec runs race to
update the same file:

  • read 1
  • read 2
  • write 1
  • write 2 - write 1 is lost

Because we need to read and write the file under the same lock,
I inline #write into #persist so that dumped_statuses is called
after the file is opened/locked.

@Empact Empact force-pushed the lock-example-status branch from fe3499f to 0344881 Compare July 15, 2015 02:20
@myronmarston
Copy link
Member

This looks reasonable. Can you add a spec for this?

@myronmarston
Copy link
Member

It also looks like some specs are failing on 1.8.7 -- let us know if you want help with that.

@Empact Empact force-pushed the lock-example-status branch 5 times, most recently from 4498948 to ad02a39 Compare July 15, 2015 19:58
@Empact
Copy link
Contributor Author

Empact commented Jul 15, 2015

@myronmarston Ok tests passing now. I'm not super-familiar with the File / IO apis but this follows the example in the File#flock docs: http://ruby-doc.org/core-2.2.2/File.html#method-i-flock.


# dumped status is called after the file is locked but before
# the output is written
allow(persister_1).to receive(:dumped_statuses) do
Copy link
Member

Choose a reason for hiding this comment

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

This can use receive(:dumped_statuses).and_wrap_original rather than capturing the method above.

https://relishapp.com/rspec/rspec-mocks/v/3-3/docs/configuring-responses/wrapping-the-original-implementation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice - I was looking for that feature. Just a sec.

@Empact Empact force-pushed the lock-example-status branch from ad02a39 to 7db676c Compare July 15, 2015 20:47
This introduces file locking to ExampleStatusPersister#persist to
avoid a race condition where parallel or unrelated spec runs race to
update the same file:

- read 1
- read 2
- write 1
- write 2 - write 1 is lost

Because we need to read and write the file under the same lock,
I inline #write into #persist so that dumped_statuses is called
after the file is opened/locked.
@Empact Empact force-pushed the lock-example-status branch from 7db676c to 7f8746f Compare July 15, 2015 20:48
myronmarston added a commit that referenced this pull request Jul 16, 2015
Lock example_status_persistence_file to prevent race conditions
@myronmarston myronmarston merged commit 8ec2c99 into rspec:master Jul 16, 2015
@myronmarston
Copy link
Member

Thanks, @Empact!

myronmarston added a commit that referenced this pull request Jul 16, 2015
JonRowe added a commit that referenced this pull request Jul 23, 2015
MatheusRich pushed a commit to MatheusRich/rspec-core that referenced this pull request Oct 30, 2020
Lock example_status_persistence_file to prevent race conditions
MatheusRich pushed a commit to MatheusRich/rspec-core that referenced this pull request Oct 30, 2020
MatheusRich pushed a commit to MatheusRich/rspec-core that referenced this pull request Oct 30, 2020
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.

2 participants