[CDRIVER-3702] Add test assertions to check lock preconditions Created: 04/Jun/20  Updated: 28/Oct/23  Resolved: 24/Jul/20

Status: Closed
Project: C Driver
Component/s: None
Affects Version/s: None
Fix Version/s: 1.18.0, 1.18.0-alpha

Type: Improvement Priority: Major - P3
Reporter: Kevin Albertson Assignee: Andrew Witten (Inactive)
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

Many functions in mongoc-topology.c, mongoc-topology-description.c, mongoc-topology-background-monitoring.c, and (after CDRIVER-3535) mongoc-server-monitor.c require callers to lock a mutex. This precondition is usually documented in a function level comment:

/*
 *--------------------------------------------------------------------------
 *
 * mongoc_topology_scan_once --
 *
 *      Runs a single complete scan.
 *
 *      NOTE: this method expects @topology's mutex to be locked on entry.
 *
 *      NOTE: this method unlocks and re-locks @topology's mutex.
 *
 *      Only runs for single threaded monitoring. (obey_cooldown is always
 *      true).
 *
 *--------------------------------------------------------------------------
 */
static void
mongoc_topology_scan_once (mongoc_topology_t *topology, bool obey_cooldown)

Though sometimes these preconditions are missed. Here is one such real life example: https://github.com/mongodb/mongo-c-driver/pull/609#discussion_r424602399

It may be worth adding test-only assertions in these functions to check that the expected mutex is locked. pthread_mutex_try_lock could accomplish this on POSIX and TryEnterCriticalSection on Windows.



 Comments   
Comment by Andrew Witten (Inactive) [ 24/Jul/20 ]

PR: https://github.com/mongodb/mongo-c-driver/pull/649

Comment by Githook User [ 24/Jul/20 ]

Author:

{'name': 'Andrew Witten', 'email': 'andrew.witten@mongodb.com', 'username': 'awitten1'}

Message: CDRIVER-3702 adds runtime debug assertions

Adds runtime debug assertions. Enable with configuration flag -DENABLE_DEBUG_ASSERTIONS=ON.
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/98fde46cad0d89afa61fc22b4845232610fc2279

Comment by Kevin Albertson [ 04/Jun/20 ]

Note, we already have test-only code compiled in for test-libmongoc via -DBSON_MEMCHECK. That define makes an ABI breaking change, and is only suitable for testing, so it is not a cmake option. We could follow suit and add another define to enable test assertions. As a note, I think there is a slight danger of adding a lot of test specific code, in that it takes the test runner a little further from testing reality. But for assertions like this, I think it is safe to do so.

Generated at Wed Feb 07 21:18:48 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.