views:

105

answers:

2

We're trying to build a class that provides the MFC CRecordset (or, really, the CODBCRecordset class) thread safety. Everything actually seem to work out pretty well for the various functions like opening and moving through the recordset (we enclose these calls with critical sections), however, one problem remains, a problem that seems to introduce deadlocks in practice.

The problem seems to lie in our constructor, like this:

CThreadSafeRecordset::CThreadSafeRecordset(void) : CODBCRecordset(g_db)
{ // <-- Deadlock!
}

The other thread might be one having ended up in CThreadSafeRecordset::Close() despite us guarding the enclosed Close call, but that doesn't really matter since the constructor is threading unaware. I assume the original CRecordset class is the culprit, doing bad things at construction time. I've looked around for programming techniques to work around this problem, but I'm unsure what could be the best solution? Since we have no code and can't control other code in our constructor, we can't wrap anything special in a critical section...?

Update: Thanks for the input; I've marked what I ended up with as the answer to my question. That, in combination with returning a shared_ptr as the returned instance for ease of updating the existing thread-unaware code.

+1  A: 

If there's no way to make CODBCRecordset move its thread-unsafe operations out of the constructor (a default constructor followed by an Initialize() call, say), you can always use composition instead of inheritance. Let CThreadSafeRecordset be a wrapper around CODBCRecordset instead of a subclass of it. That way, you can explicitly construct your recordset whenever you like, and can defend it with whatever rigor is appropriate.

The drawback, of course, is that you'll have to wrap every CODBCRecordset method you wish to expose, even the ones that don't relate to your threading guarantees. A C macro in the cpp file (so that it can't escape and afflict your clients) may help.

David Seiler
+2  A: 

You can make the CThreadSafeRecordset constructor private, then provide a public factory method that participates in your locking and returns an instance.

Aidan Ryan