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(processor): remove race condition failsafe #4791

Merged
merged 3 commits into from Oct 8, 2021

Conversation

SukkaW
Copy link
Member

@SukkaW SukkaW commented Oct 2, 2021

What does it do?

This closes #4430

How to test

git clone -b fix-race-condition https://github.com/sukkaw/hexo.git
cd hexo
npm install
npm test

Pull request tasks

  • Add test cases for the changes.
  • Passed the CI test.

@coveralls
Copy link

coveralls commented Oct 2, 2021

Coverage Status

Coverage increased (+0.002%) to 98.137% when pulling 79adad1 on SukkaW:fix-race-condition into 9edbc99 on hexojs:master.

Comment on lines -166 to 172
// FIXME: Data may be inserted when reading files. Load it again to prevent
// race condition. We have to solve this in warehouse.
const doc = Post.findOne({source: file.path});

if (doc) {
if (file.type !== 'update') {
ctx.log.warn(`Trying to "create" ${magenta(file.path)}, but the file already exists!`);
}
return doc.replace(data);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

There are only three reasons that a doc will already exist while the processor is trying to create a post:

  • A race condition inside the warehouse (which seems to be fixed in warehouse@2.1.0 back in 2015)
  • The file.type is update. in this case, there is no need to load the doc again.
  • In some rare situation where two file object is created, with the same id and the same create type (E.g. inside our unit test when we trying to create 2 files under the same file.path. This is not likely to happen in the real-world production). But Hexo can still deal with the issue by replacing doc directly. And I add a warning message for the case, and hopefully, someone who encounters the issue would report it to us.

Comment on lines +314 to +317
return Promise.all([
post.remove(),
unlink(file.source)
]);
Copy link
Member Author

Choose a reason for hiding this comment

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

Both _Document.remove method and fs.unlink method returns Promise.

@SukkaW SukkaW requested a review from a team October 2, 2021 13:30
@SukkaW SukkaW merged commit 02cbfe3 into hexojs:master Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Post Processor: about race condition
3 participants