Skip to content

SharedChan is not Freeze #11039

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

Closed
metajack opened this issue Dec 18, 2013 · 6 comments · Fixed by #11111 or #11315
Closed

SharedChan is not Freeze #11039

metajack opened this issue Dec 18, 2013 · 6 comments · Fixed by #11111 or #11315
Labels
E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added.

Comments

@metajack
Copy link
Contributor

The recent rewrite of std::comm had one major side effect not mentioned in the notes. SharedChan is no longer Freeze. This breaks a lot of code in Servo.

cc @alexcrichton

@metajack
Copy link
Contributor Author

Ok, That was a bit of an overreaction. So far this broke the Font structure which had a SharedChan to send profile data to the profiler task. This turned out to be unused, but I expect I will run into elsewhere too, and it's not so easily removed unless we want to redo the profiler.

@thestinger
Copy link
Contributor

I don't think there's much use for a Freeze bound in most cases. For example, an Arc needs to make sure that the contained object can be shared between threads without extra synchronization, and using Freeze is overly strict. It would be nice to replace it with another kind more targeted at thread-safety.

@metajack
Copy link
Contributor Author

In this case I was trying to use it with RefCell. The other uses of the profiler SharedChan may not be in RefCells and may not run into this. I'm still not done with the port, so I will update if I find more problems.

@alexcrichton
Copy link
Member

Hm, as I think about this, this may be a good thing. Chan should definitely not be Freeze, and although an instance of SharedChan is in theory Freeze, sharing should be achieved by cloning instead of through an arc, so this may be working-as-intended?

@metajack
Copy link
Contributor Author

Apparently it is also not Send.

@alexcrichton
Copy link
Member

I'm going to think of this closed as working-as-intended but instead flag this as needstest for now.

All of Port Chan and SharedChan cannot be Freeze in the sense that they can have shareable read-only copies among tasks. It's my understanding that this is what Freeze means today for at least MutexArc.

bors added a commit that referenced this issue Dec 23, 2013
None of these primitives should be Freeze because sharing them in an Arc is a
very bad idea.

Closes #11039
bors added a commit that referenced this issue Jan 6, 2014
There was a scheduling race where a child may not increment the global task
count before the parent exits, and the parent would then think that there are no
more tasks left.

Closes #11039
flip1995 pushed a commit to flip1995/rust that referenced this issue Jun 30, 2023
Add `BLESS` for compile-test and some cleanup

changelog: none

Allows passing the environment variable `BLESS` to bless tests, which is useful when you want to bless internal tests - `BLESS= cargo uitest -Finternal`

Also updates a place in the docs referring to `cargo dev bless` and removes some unused test deps
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added.
Projects
None yet
3 participants