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

fixed race condition on the main map, added a test, normalized filepaths, handled windows rename issue #3

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

chrhlnd
Copy link

@chrhlnd chrhlnd commented Oct 12, 2015

The main map had a race condition. Made a minor test file for ya. Made the test pass on windows, which had issues with your rename. Also normalized the paths with the filepath package.

@sdomino
Copy link
Owner

sdomino commented Oct 13, 2015

@chrhlnd, thanks for the pull request. If you could do a couple of things, that would be great:

  • Pull the latest master to make sure everything is up to date (there are currently some conflicts)
  • Create a branch for your changes so when they are ready to be merged we can squash them
  • Integrate your tests into the tests that are currently on master.

Your changes look good, and I'm confident we'll get them merged in, thanks.

c, ok := d.mutexes[collection]

d.maplock.RUnlock()
Copy link
Contributor

Choose a reason for hiding this comment

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

dropping the read lock and acquiring a write lock is a race condition. Inside the write lock you should check to see if the mutex has already been created, to avoid a second mutex being created.

@chrhlnd
Copy link
Author

chrhlnd commented Oct 13, 2015

Re-pushed with the check.

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.

3 participants