reclaim does not invalidate the passed reference

[guidola]

Hi,

first of all, nice work on the library it is looking pretty well so far 👍

Looking at reclaim method for the boltDriverPool type and considering that u meant to "invalidate" the passed reference, noticed that although you were storing a new instance for the booltConn passed as a pointer to be returned to the pool therefore preventing that the instance queued to the channel has any external reference, on the passed instance still holds the same connection information than the queued one. I see the passed pointer is set to null at the end but that just prevents access on the local scope, on outer scopes there might be other pointers with the same reference value and therefore still holding the queued connection information which could end on different go-routines holding the same connection information.

I think, changing conn = nil for something like *conn = boltConn{} or conn.closed = true would prevent that behavior. First allocates non necessary memory and second kind of depends on internal handling of connections marked as closed. Also it could just be marked as a postcondition for the use of reclaim that reclaimed connections cannot be used further in the code.

Although i do not completely know if that was the intention, just sharing some thoughts I had while reading the code.

Thanks.

[johnnadratowski]

@guidola - Thank you for the issue submission.

I cannot set conn.closed = true as the point of reclaim is to be able to reuse these connections. Perhaps I could hack around it to set conn.closed = false before it is claimed again, however.

In looking at the code, I think you're right, I made a mistake there. I should have probably set *conn = boltConn{} instead of setting the reference. I'm going to try to produce a invalid behavior for this in a test.

Thanks again.

[guidola]

@johnnadratowski Glad to be of help.

Although, i think it is actually possible setting the value of closed to true for the passed instance since the memory spaces of this one and the one stored on the channel are not the same hence changes made on the attributes of one instance are not reflected on the other one. This way the instance on the channel is marked as open and the one from the caller is marked as closed.

Also, i've been building a wrapper around the pooling system to get a certain behavior that i think could be great as something native and seems to fit pretty well with your implementation. So if you're interested i can open an issue for it and discuss it over there.

[johnnadratowski]

@guidola - Please do, I'm very interested to see what you've been working on, and would be happy to work with you to get it included if it fits with the project goals. I'll hold off on looking at your original request until I see your PR. Please just make sure you add tests for your new stuff. They can be high-level as I did the other pooling connection tests.