Skip to content

Conversation

@chkothe
Copy link
Contributor

@chkothe chkothe commented Oct 30, 2020

This is a merger of #70 along with the queue fix from #71 (and some tweaks).

@tstenner
Copy link
Collaborator

tstenner commented Nov 2, 2020

So far, looking good. The locking seems fine to me and the benchmark that exhibited the crash / infinite loop runs through just fine.
Two things: there are a bunch of indentation changes that make it hard to see what changed (or who changed a line last with git blame) and the commit authorship didn't survive the rebasing.

I've split the PR into two commits (@jfrey-xx's changes and yours) and once the CI is happy I'll merge it.

@jfrey-xx
Copy link
Contributor

jfrey-xx commented Nov 2, 2020

Thanks a lot for bringing in the changes! It will for sure help a lot here :) Very minor remark, but since I noticed it: in the changelog from tstenner@340d517 I believe that the race condition is actually from #71 ^^

@tstenner
Copy link
Collaborator

tstenner commented Nov 2, 2020

Thanks a lot for bringing in the changes! It will for sure help a lot here :) Very minor remark, but since I noticed it: in the changelog from tstenner@340d517 I believe that the race condition is actually from #71 ^^

fixed

@tstenner tstenner closed this Nov 2, 2020
@chkothe
Copy link
Contributor Author

chkothe commented Nov 2, 2020

Ah sorry, my editor was still set to indent with 4 spaces instead of tabs.

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