-
-
Notifications
You must be signed in to change notification settings - Fork 753
Lock example_status_persistence_file to prevent race conditions #2029
Conversation
fe3499f
to
0344881
Compare
This looks reasonable. Can you add a spec for this? |
It also looks like some specs are failing on 1.8.7 -- let us know if you want help with that. |
4498948
to
ad02a39
Compare
@myronmarston Ok tests passing now. I'm not super-familiar with the File / IO apis but this follows the example in the |
|
||
# dumped status is called after the file is locked but before | ||
# the output is written | ||
allow(persister_1).to receive(:dumped_statuses) do |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
ad02a39
to
7db676c
Compare
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.
7db676c
to
7f8746f
Compare
Lock example_status_persistence_file to prevent race conditions
Thanks, @Empact! |
Lock example_status_persistence_file to prevent race conditions
[skip ci]
This introduces file locking to ExampleStatusPersister#persist to
avoid a race condition where parallel or unrelated spec runs race to
update the same file:
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.