postgresql-libpq icon indicating copy to clipboard operation
postgresql-libpq copied to clipboard

Possibility of memory leaks with asynchronous exceptions

Open arybczak opened this issue 10 years ago • 5 comments

Any code that follows the pattern "allocate bare resource using a function imported from C, then create a foreign pointer that manages the resource" needs to mask asynchronous exceptions, since if exception arrives between allocation and wrapping, memory leaks. This is particularly dangerous in connectdb, since such code may eventually run out of reachable database connections, completely blocking the access to the database.

arybczak avatar Dec 28 '14 22:12 arybczak

You are correct, and this issue is one that I am well aware of. I just haven't gotten around to reviewing and improving the library... and there's a lot to review.

I'm not quite sure what you mean by "reachable database connections", and blocking access to the database however. Unless you mean leaking an open database connection, and eventually running into the backend's configured connection limit.

lpsmith avatar Dec 28 '14 22:12 lpsmith

Unless you mean leaking an open database connection, and eventually running into the backend's configured connection limit.

Yes, that's exactly it.

arybczak avatar Dec 28 '14 22:12 arybczak

Ok, well I pushed a fix for #26, how much do you want to fix before I release to hackage?

I would be willing to review and fix the connection functions myself. I don't really want to go through the rest of the library at the moment... I'll probably get around to it someday, but honestly it's not a personal priority at the moment.

lpsmith avatar Dec 28 '14 23:12 lpsmith

It looks like securing PGconn, PGresult and PGcancel is easily doable by masking connectdb, connectStart, resultFromConn and getCancel functions, so if you could do that it would be great. I see there are some functions using malloc/free directly that are more subtle to localize and fix. I might actually look at them later myself and secure them appropriately, but it's not critical as I don't use them at the moment.

Btw, thanks for the very fast response!

arybczak avatar Dec 29 '14 01:12 arybczak

Err, I don't recall closing this issue, I don't understand why I would have. :-/

lpsmith avatar Aug 26 '16 06:08 lpsmith