Skip to content

Remove invalid spsc_queue test #16797

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
Aug 27, 2014

Conversation

nikomatsakis
Copy link
Contributor

This test seems to read freed memory -- the peeked variable points into the queue, but then the pop operation removes the node from the queue and moves the enclosing T elsewhere, invalidating the peeked pointer.

r? @alexcrichton

@nikomatsakis
Copy link
Contributor Author

@pnkfelix this seems to be the problem we were examining.

bors added a commit that referenced this pull request Aug 27, 2014
…st, r=alexcrichton

This test seems to read freed memory -- the peeked variable points into the queue, but then the pop operation removes the node from the queue and moves the enclosing `T` elsewhere, invalidating the `peeked` pointer.

r? @alexcrichton
@pnkfelix
Copy link
Member

What a strange test. Why was it added in the first place? Should we consider incorporating it with some sort of ensure_capacity method to ensure that the backing storage is not freed and replaced during the test? Or is it just a waste?

@bors bors closed this Aug 27, 2014
@bors bors merged commit 5c82f48 into rust-lang:master Aug 27, 2014
@nikomatsakis
Copy link
Contributor Author

On Wed, Aug 27, 2014 at 01:22:53PM -0700, Felix S Klock II wrote:

What a strange test. Why was it added in the first place? Should
we consider incorporating it with some sort of ensure_capacity
method to ensure that the backing storage is not freed and replaced
during the test? Or is it just a waste?

It's unclear to me what value this test has, but the comment does not
suggest any intent to test the ensure_capacity method.

@nikomatsakis nikomatsakis deleted the remove-invalid-spsc_queue-test branch March 30, 2016 16:17
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.

4 participants