-
Notifications
You must be signed in to change notification settings - Fork 470
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
Port Golang chan1.go Test #615
Port Golang chan1.go Test #615
Conversation
crossbeam-channel/tests/golang.rs
Outdated
panic!("fail") | ||
} | ||
data[r] = 2; | ||
drop(data); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Someone who is more experienced with Rust than I am probably knows why, but this blocks unless I manually drop.
Any thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There may be an odd interaction with select macro. I'll take a look.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@taiki-e What do I need to do to get this PR merged in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if I remove drop(data)
in the r
function, I couldn't reproduce block. I can reproduce block by removing the drop(data)
in the s
function, but it will not block if I disable parallel tests (cargo test -- --test-threads=1
) or run the test individually (cargo test --test golang -- chan1::main
).
@SkamDart Could you remove drop(data)
in the r
function and leave the link to this comment on the drop(data)
in the s
function as a comment? (Otherwise, LGTM)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! LGTM aside from a nit. I'll merge when a suggested change is applied.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bors r+
Build succeeded! And happy new year! 🎉 |
Rust port chan1.go
Based on an ask in #201