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

Try to fix #3075 #3081

Merged
merged 5 commits into from
Jul 11, 2022
Merged

Try to fix #3075 #3081

merged 5 commits into from
Jul 11, 2022

Conversation

durban
Copy link
Contributor

@durban durban commented Jul 7, 2022

This PR tries to fix #3075. It is on top of #3074.

I've left alone two while loops:

  • One of them is in a blocking call, so it seems ok.
  • The other one loops while holding the semaphore. It seemed dangerous to break it up.

Hopefully there are no semantic changes (I've moved a manyDone.set(true) from a finally, which might be a problem. But I didn't understand why it was in a finally...).

@armanbilge
Copy link
Member

I cancelled CI because it was hanging. But this is a problem you've inherited from #3074 :)

@durban
Copy link
Contributor Author

durban commented Jul 7, 2022

Ok, thank you. Do we know why that happens?

@armanbilge
Copy link
Member

I don't, but maybe @djspiewak has a hunch?

@durban
Copy link
Contributor Author

durban commented Jul 8, 2022

Yeah, it's a real deadlock. Let's see if this fixes it...

@armanbilge
Copy link
Member

Hey, looking good, nice work! 😃

@djspiewak
Copy link
Member

Ah, I just got to this one! :-) You found the same bug that I found. It's fixed in the upstream PR now. I think that fixes everything? Let's see.

@armanbilge
Copy link
Member

@durban do you think you could merge the latest #3074 into your fix for #3075? Thanks!

@durban
Copy link
Contributor Author

durban commented Jul 10, 2022

@djspiewak Yes, I think so too.

@armanbilge Done.

@djspiewak djspiewak merged commit badc924 into typelevel:series/3.3.x Jul 11, 2022
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