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

Recycled values cannot contain references to other values from the same Pool #17

Closed
zslayton opened this Issue Nov 8, 2018 · 1 comment

Comments

Projects
None yet
1 participant
@zslayton
Owner

zslayton commented Nov 8, 2018

The Drop implementation for the RecycledInner type is responsible for returning the value being dropped to the Pool that originally issued it.

The insert_or_drop method performs two operations:

  1. If the CappedCollection has available capacity, the provided value will be reset() and then added to the Pool again.
  2. If it does not have capacity, the value will be drop()ed.

If the value contains other values from the same Pool (e.g. it's a nested container, like a Vec that indirectly contains other Vecs), the call to reset() in step 1 will cause the program to panic!(). This happens because the call to insert_or_drop uses a mutable borrow of the RefCell<CappedCollection<_>>. A nested call to insert_or_drop will also try to mutably borrow the RefCell<CappedCollection<_>>, causing RefCell's runtime validation logic to panic!().

This can be fixed by refactoring the Drop implementation to:

  • Immutably borrow the RefCell<CappedCollection<_>> to test its available capacity.
    • If there's no space available, drop the value.
    • If there is space available, call reset() on the value being returned to the Pool. (Notice that we're now resetting it without holding a mutable reference to the RefCell<CappedCollection<_>>.) Then mutably borrow the collection and add the value to it.

zslayton pushed a commit that referenced this issue Nov 13, 2018

@zslayton

This comment has been minimized.

Owner

zslayton commented Nov 13, 2018

Fixed by 5bd809b.

@zslayton zslayton closed this Nov 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment