Skip to content

Commit 6414b60

Browse files
committed
Fix of the last merge.
Plus slightly optimized allocating/deallocating of internally used Stmt handles.
1 parent e61572c commit 6414b60

File tree

7 files changed

+119
-109
lines changed

7 files changed

+119
-109
lines changed

driver/ma_api_internal.cpp

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -279,21 +279,19 @@ SQLRETURN MA_SQLCancel(SQLHSTMT StatementHandle)
279279

280280
// Technically, if the application does not do good syncronization of stmt use here, we can have
281281
// Stmt already freed at this point, and crash right away on reading the mutex in Stmt
282-
EnterCriticalSection(&globalLock);
282+
std::unique_lock<std::mutex> localScopeLock(globalLock);
283283
if (CheckDeletedStmt(Stmt) != NULL)
284284
{
285-
LeaveCriticalSection(&globalLock);
286285
return ret;// i.e. SQL_SUCCESS. Stmt has been deleted, nothing to cancel already
287286
}
288-
if (!TryEnterCriticalSection(&Stmt->CancelDropSwitch))
287+
if (!Stmt->CancelDropSwitch.try_lock())
289288
{
290-
LeaveCriticalSection(&globalLock);
291289
// This is not clear what is right here - success or error
292290
return SQL_SUCCESS;
293291
}
294292
// We can release the lock. SQL_DROP will get it, but stop at the Stmt->CancelDropSwitch
295293
// that we already own
296-
LeaveCriticalSection(&globalLock);
294+
localScopeLock.unlock();
297295
MADB_CLEAR_ERROR(&Stmt->Error);
298296

299297
MDBUG_C_ENTER(Stmt->Connection, "SQLCancel");
@@ -337,7 +335,7 @@ SQLRETURN MA_SQLCancel(SQLHSTMT StatementHandle)
337335
} // Else we are canceling function running in other thread.
338336
else if (lock.try_lock())
339337
{
340-
Stmt->canceled= '\1';
338+
Stmt->canceled= true;
341339
try
342340
{
343341
/* "If a SQL statement is being executed when SQLCancel is called on another thread to cancel the
@@ -362,7 +360,7 @@ SQLRETURN MA_SQLCancel(SQLHSTMT StatementHandle)
362360
ret= MADB_KillAtServer(Stmt->Connection, &Stmt->Error);
363361
}
364362
}
365-
LeaveCriticalSection(&Stmt->CancelDropSwitch);
363+
Stmt->CancelDropSwitch.unlock();
366364
MDBUG_C_RETURN(Stmt->Connection, ret, &Stmt->Error);
367365
}
368366
/* }}} */

driver/ma_driver.cpp

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@
2424
*/
2525
/************************************* Driver wide stuff ************************************************/
2626

27-
CRITICAL_SECTION globalLock;
27+
std::mutex globalLock;
2828
static MADB_List *deletedStmt= NULL;
29-
static unsigned int envCount= 0;
29+
static std::atomic_uint32_t envCount(0U);
3030

3131
#ifndef _WIN32
3232
__attribute__((constructor))
@@ -43,53 +43,48 @@ extern "C" {
4343
/* {{{ DriverGlobalInit */
4444
void DriverGlobalInit()
4545
{
46-
InitializeCriticalSection(&globalLock);
4746
}
4847
/* }}} */
4948

5049
/* {{{ DriverGlobalClean()*/
5150
void DriverGlobalClean(void)
5251
{
53-
EnterCriticalSection(&globalLock);
52+
// There is no need to lock here at least the while it used the way it used now -
53+
// only called when library is unloaded
5454
if (deletedStmt)
5555
{
5656
MADB_ListFree(deletedStmt, FALSE);
5757
}
58-
LeaveCriticalSection(&globalLock);
59-
DeleteCriticalSection(&globalLock);
6058
}
6159
/* }}} */
6260
}
6361
/* {{{ IncrementEnvCount */
6462
// Normally there should be 1 Env, but nothing forbids app have more than 1.
6563
void IncrementEnvCount()
6664
{
67-
EnterCriticalSection(&globalLock);
6865
++envCount;
69-
LeaveCriticalSection(&globalLock);
7066
}
7167
/*}}}*/
7268

7369
/* {{{ DecrementEnvCount */
7470
// If the last Env has been freed - we should probably clean the list
7571
void DecrementEnvCount()
7672
{
77-
EnterCriticalSection(&globalLock);
7873
--envCount;
7974
if (!envCount)
8075
{
76+
std::lock_guard<std::mutex> localScopeLock(globalLock);
8177
MADB_ListFree(deletedStmt, FALSE);
8278
deletedStmt= NULL;
8379
}
84-
LeaveCriticalSection(&globalLock);
8580
}
8681
/*}}}*/
8782

8883
/* {{{ CheckDeletedStmt */
8984
// If the last Env has been freed - we should probably clean the list
9085
MADB_List* CheckDeletedStmt(void* stmtObjAddr)
9186
{
92-
MADB_List* item= deletedStmt;
87+
MADB_List *item= deletedStmt;
9388
while (item != NULL)
9489
{
9590
if (item->data == stmtObjAddr)
@@ -104,18 +99,17 @@ MADB_List* CheckDeletedStmt(void* stmtObjAddr)
10499

105100
/* {{{ RemoveStmtFromDeleted */
106101
// If the last Env has been freed - we should probably clean the list
107-
BOOL RemoveStmtFromDeleted(void* stmtObjAddr)
102+
bool RemoveStmtFromDeleted(void* stmtObjAddr)
108103
{
109-
BOOL result= FALSE;
110-
EnterCriticalSection(&globalLock);
104+
bool result= false;
105+
std::lock_guard<std::mutex> localScopeLock(globalLock);
111106
MADB_List* found= CheckDeletedStmt(stmtObjAddr);
112107
if (found)
113108
{
114109
deletedStmt= MADB_ListDelete(deletedStmt, found);
115110
free(found);
116-
result= TRUE;
111+
result= true;
117112
}
118-
LeaveCriticalSection(&globalLock);
119113
return result;
120114
}
121115
/*}}}*/

driver/ma_driver.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,15 @@
3030
#ifndef _ma_driver_h_
3131
#define _ma_driver_h_
3232

33-
extern CRITICAL_SECTION globalLock;
33+
extern std::mutex globalLock;
3434
extern "C" {
3535
void DriverGlobalInit();
3636
void DriverGlobalClean(void);
3737
}
3838
void IncrementEnvCount();
3939
void DecrementEnvCount();
4040
MADB_List* CheckDeletedStmt(void* stmtObjAddr);
41-
BOOL RemoveStmtFromDeleted(void* stmtObjAddr);
41+
bool RemoveStmtFromDeleted(void* stmtObjAddr);
4242
void RememberDeletedStmt(void* stmtObjAddr);
4343

4444
typedef struct {

driver/ma_helper.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1141,10 +1141,11 @@ SQLRETURN MADB_DaeStmt(MADB_Stmt *Stmt, SQLUSMALLINT Operation)
11411141
MADB_CLEAR_ERROR(&Stmt->Error);
11421142

11431143
if (Stmt->DaeStmt)
1144-
Stmt->Methods->StmtFree(Stmt->DaeStmt, SQL_DROP);
1145-
Stmt->DaeStmt= nullptr;
1144+
{
1145+
MADB_DeleteDaeStmt(Stmt);
1146+
}
11461147

1147-
if (!SQL_SUCCEEDED(MA_SQLAllocHandle(SQL_HANDLE_STMT, (SQLHANDLE)Stmt->Connection, (SQLHANDLE *)&Stmt->DaeStmt)))
1148+
if (!SQL_SUCCEEDED(MADB_StmtInit(Stmt->Connection, (SQLHANDLE *)&Stmt->DaeStmt, false)))
11481149
{
11491150
return MADB_CopyError(&Stmt->Error, &Stmt->Connection->Error);
11501151
}
@@ -1183,7 +1184,7 @@ SQLRETURN MADB_DaeStmt(MADB_Stmt *Stmt, SQLUSMALLINT Operation)
11831184
if (!SQL_SUCCEEDED(Stmt->DaeStmt->Prepare(Query.c_str(), (SQLINTEGER)Query.length(), true)))
11841185
{
11851186
MADB_CopyError(&Stmt->Error, &Stmt->DaeStmt->Error);
1186-
Stmt->Methods->StmtFree(Stmt->DaeStmt, SQL_DROP);
1187+
MADB_DeleteDaeStmt(Stmt);
11871188
}
11881189
return Stmt->Error.ReturnValue;
11891190
}

driver/ma_odbc.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -287,13 +287,14 @@ struct MADB_Env {
287287
// Stmt has to know few things about connection and descriptor
288288
#include "ma_connection.h"
289289
#include "ma_desc.h"
290-
290+
#define xxx sizeof(std::mutex)
291291
struct MADB_Stmt
292292
{
293293
MADB_QUERY Query;
294294
MADB_StmtOptions Options;
295295
MADB_Error Error;
296-
CRITICAL_SECTION CancelDropSwitch; /* mutex for SQLCancel/SQLFreeStmt(SQL_DROP) */
296+
std::mutex CancelDropSwitch; /* mutex for SQLCancel/SQLFreeStmt(SQL_DROP) */
297+
std::atomic_bool canceled;
297298
MADB_Cursor Cursor;
298299
MADB_List ListItem;
299300
long long AffectedRows= 0;
@@ -339,7 +340,6 @@ struct MADB_Stmt
339340
bool PositionedCommand= false;
340341
bool RebindParams= false;
341342
bool bind_done= false;
342-
std::atomic_bool canceled;
343343

344344
MADB_Stmt(MADB_Dbc *Connection);
345345
SQLRETURN Prepare(const char* StatementText, SQLINTEGER TextLength, bool ServerSide= true, bool DirectExecution= false);

0 commit comments

Comments
 (0)