Skip to content

Commit

Permalink
Fix a race condition if PathRequest::doCreate races with the path bei…
Browse files Browse the repository at this point in the history 
…ng processed.
  • Loading branch information
@JoelKatz
JoelKatz committed Nov 8, 2013
1 parent 49f43cc commit 4620b66
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/ripple_app/paths/PathRequest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ Json::Value PathRequest::doCreate (Ledger::ref lrLedger, const Json::Value& valu
}
else
mValid = false;

status = jvStatus;
}

if (mValid)
Expand All @@ -136,7 +138,7 @@ Json::Value PathRequest::doCreate (Ledger::ref lrLedger, const Json::Value& valu
sRequests.insert (shared_from_this ());
}

return jvStatus;
return status;
}

int PathRequest::parseJson (const Json::Value& jvParams, bool complete)
Expand Down

3 comments on commit 4620b66

@vinniefalco
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm surprised that a race condition is fixed with this microscopic change. And less surprised that there are no supporting comments.

@JoelKatz
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's obvious what the code does -- it saves the status inside the object in a local variable which is then returns. The previous code didn't do this. That there's a possible race condition is an artifact of the code history. When you create an object, you have exclusive use of it and don't need any locks until you put it where other code can find it. After that, you have to follow its locking rules.

@vinniefalco
Copy link
Contributor

Choose a reason for hiding this comment

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

Obvious to you

Please sign in to comment.