Skip to content
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

RefCell<T> already borrowed #34

Closed
radford opened this issue Apr 6, 2014 · 4 comments
Closed

RefCell<T> already borrowed #34

radford opened this issue Apr 6, 2014 · 4 comments

Comments

@radford
Copy link
Contributor

radford commented Apr 6, 2014

I'm seeing pool.get_connection() return connections where the RefCell is WRITING (as opposed to UNUSED like it should be), so that the first use of the connection fails with RefCell<T> already borrowed. If I sleep(1) after getting the connection, then it becomes UNUSED, so it seems like a race condition.

This only happens when the client is cond.wait()ing in get_connection(), so that it gets a connection that was just previously used.

@sfackler
Copy link
Owner

sfackler commented Apr 6, 2014

My gut feeling is that it's the same root cause as #31: rust-lang/rust#13246.

@radford
Copy link
Contributor Author

radford commented Apr 6, 2014

Your gut feeling is correct. My code is equivalent to the following

extern crate postgres;
fn main() {
  let pool = postgres::pool::PostgresConnectionPool::new("postgres://localhost/postgres", postgres::NoSsl, 1).unwrap();
  let _ : Vec<i32> = {
    let conn = pool.clone().get_connection();
    let stmt = conn.prepare("SELECT 1 UNION SELECT 2").unwrap();
    let rows = stmt.query([]).unwrap();
    rows.map(|r| {
      r[1]
    }).collect()
  };
}

which currently will drop the PostgresStatement then the PooledPostgresConnection and then incorrectly the PostgresRows. This means that the access to the connection while dropping the rows will race with anyone using the released connection. Ouch.

@sfackler
Copy link
Owner

sfackler commented Apr 6, 2014

You can work around it by splitting the last statement to

let v = rows.map(|r| r[1]).collect();
v

I might redo the internals a bit to store an Rc of the thing they have a borrowed pointer to to force the destructors to run in the correct order until the underlying issue is fixed.

sfackler added a commit that referenced this issue Apr 8, 2014
This is a workaround for rust-lang/rust#13246 to prevent total badness
until it gets fixed.

cc #34, #31
@sfackler
Copy link
Owner

I believe this should be fixed by rust-lang/rust#21972.

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

No branches or pull requests

2 participants