|Keep It Simple, Stupid|
Beware of global variables, for they may prevent your destruction.by kyle (Abbot)
|on Nov 25, 2009 at 20:03 UTC||Need Help??|
In Can DBI modify queries on the fly?, I was looking for a way to insert comments into the SQL that's sent to our app's database so that the DB's log could help us find problems in the code. Through the helpful hints here, I eventually came to a general solution, which I implemented in a branch, tested with our (poor) automated tests, and put out for our QA department to look at next.
In the SQL comments, I wanted information about our current request, but I couldn't see a way, from a DBI root module, to get that object. This isn't literally the original Apache request object but our own wrapper around it, which is passed around a lot but not globally accessible. Naturally it is not passed to any DBI function, where I wanted to use it.
So I stuck it in a global (package) variable.
Problem solved! When we create the object in the main request handler (which dispatches all over the place), a reference is stored in a global variable, and I can get that variable from the DBI code elsewhere.
Do you see the bug I had just created?
The request object we use does some bookkeeping when it gets destroyed at the end of each request. Since I'd made a permanent reference to it in a package variable, it never got destroyed, and that important end-of-life action never happened. This caused some fairly strange bugs.
The solution is simple enough.
Now the package variable gets turned to undef when the original gets destroyed, and it doesn't prevent the destruction anymore.
My lesson from this experience, I'd sum up this way: When putting a lexically scoped reference into a package variable, consider making it a weak reference. That could probably go not only for package variables, but any variable with a scope larger than the original. I don't expect that using Scalar::Util::weaken is always the best thing to do, but it will always be worth thinking about.