Skip to content

Don't call drop when taking the address of unsized fields #25595

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

Merged
merged 1 commit into from
May 20, 2015

Conversation

dotdash
Copy link
Contributor

@dotdash dotdash commented May 19, 2015

When taking the address of an unsized field we generate a rvalue datum
for the field and then convert it to an lvalue datum. At that point,
cleanup is scheduled for the field, leading to multiple drop calls.

The problem is that we generate an rvalue datum for the field, since the
pointer does not own the data and there's already cleanup scheduled
elsewhere by the true owner. Instead, an lvalue datum must be created.

Thanks to @eddyb for identifying the underlying cause and suggesting the
correct fix.

Fixes #25549.

@rust-highfive
Copy link
Contributor

r? @pcwalton

(rust_highfive has picked a reviewer for you, use r? to override)

@bluss
Copy link
Member

bluss commented May 19, 2015

This should fix #25515 too? And the use of the Drop trait in the reproduction and in the test is incidental, isn't it? A bit confusing, any trait will do.

@eddyb
Copy link
Member

eddyb commented May 19, 2015

LGTM, but the test could use also checking another arbitrary trait, I believe @bluss is correct here.

@pnkfelix
Copy link
Member

(or maybe test the case with no trait at all for a sized type, as illustrated by @Thiez here , if you like...)

When taking the address of an unsized field we generate a rvalue datum
for the field and then convert it to an lvalue datum. At that point,
cleanup is scheduled for the field, leading to multiple drop calls.

The problem is that we generate an rvalue datum for the field, since the
pointer does not own the data and there's already cleanup scheduled
elsewhere by the true owner. Instead, an lvalue datum must be created.

Thanks to @eddyb for identifying the underlying cause and suggesting the
correct fix.

Fixes rust-lang#25549
Fixes rust-lang#25515
@dotdash
Copy link
Contributor Author

dotdash commented May 19, 2015

@bors r=eddyb

@bors
Copy link
Collaborator

bors commented May 19, 2015

📌 Commit b802b18 has been approved by eddyb

bors added a commit that referenced this pull request May 20, 2015
When taking the address of an unsized field we generate a rvalue datum
for the field and then convert it to an lvalue datum. At that point,
cleanup is scheduled for the field, leading to multiple drop calls.

The problem is that we generate an rvalue datum for the field, since the
pointer does not own the data and there's already cleanup scheduled
elsewhere by the true owner. Instead, an lvalue datum must be created.

Thanks to @eddyb for identifying the underlying cause and suggesting the
correct fix.

Fixes #25549.
@bors
Copy link
Collaborator

bors commented May 20, 2015

⌛ Testing commit b802b18 with merge 6d718f2...

@bors bors merged commit b802b18 into rust-lang:master May 20, 2015
@dotdash dotdash deleted the issue25549 branch July 27, 2015 08:49
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.

Objects can be dropped an arbitrary number of times in safe code
7 participants