Skip to content

Commit

Permalink
ODBC-280 The fix of possible crash in threaded environment.
Browse files Browse the repository at this point in the history
The crash could occur if 2 connections established at same time in
different threads. One of functions read/wrote to the same memory, and
it's call wasn't guarded. The fix actually completely removed it from
connection routine, because it's not really needed to be re-run for each
connecion. Now it part of env init, and also the call is guarded.
Some warnings fixes on Windows.
  • Loading branch information
lawrinn committed Apr 26, 2020
1 parent bc35428 commit 2b420d7
Show file tree
Hide file tree
Showing 9 changed files with 38 additions and 23 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ IF(WIN32)
# SET(DSN_DIALOG_FILES ${DSN_DIALOG_FILES}
# ma_platform_win32.c)

SET(PLATFORM_DEPENDENCIES ws2_32 Shlwapi)
SET(PLATFORM_DEPENDENCIES ws2_32 Shlwapi Pathcch)
IF (MSVC)
MESSAGE(STATUS "MSVC_VERSION= ${MSVC_VERSION}")
IF (NOT(MSVC_VERSION LESS 1900))
Expand Down
9 changes: 5 additions & 4 deletions ma_connection.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
*************************************************************************************/
#include <ma_odbc.h>

extern const char* DefaultPluginLocation;

struct st_madb_isolation MADB_IsolationLevel[] =
{
{SQL_TRANSACTION_REPEATABLE_READ, "REPEATABLE READ"},
Expand Down Expand Up @@ -140,7 +142,7 @@ SQLRETURN MADB_DbcSetAttr(MADB_Dbc *Dbc, SQLINTEGER Attribute, SQLPOINTER ValueP
if (!Dbc)
{
/* Todo: check */
if (Attribute != SQL_ATTR_TRACE ||
if (Attribute != SQL_ATTR_TRACE &&
Attribute != SQL_ATTR_TRACEFILE)
return SQL_INVALID_HANDLE;
return SQL_SUCCESS;
Expand Down Expand Up @@ -609,10 +611,9 @@ SQLRETURN MADB_DbcConnectDB(MADB_Dbc *Connection,
}
else
{
const char *DefaultLocation= MADB_GetDefaultPluginsDir(Connection);
if (DefaultLocation != NULL)
if (DefaultPluginLocation != NULL)
{
mysql_optionsv(Connection->mariadb, MYSQL_PLUGIN_DIR, DefaultLocation);
mysql_optionsv(Connection->mariadb, MYSQL_PLUGIN_DIR, DefaultPluginLocation);
}
}

Expand Down
2 changes: 1 addition & 1 deletion ma_connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ SQLRETURN MADB_Dbc_GetCurrentDB(MADB_Dbc *Connection, SQLPOINTER CurrentDB, SQLI
SQLSMALLINT *StringLengthPtr, my_bool isWChar);
BOOL MADB_SqlMode(MADB_Dbc *Connection, enum enum_madb_sql_mode SqlMode);
/* Has platform versions */
const char* MADB_GetDefaultPluginsDir(MADB_Dbc *Dbc);
char* MADB_GetDefaultPluginsDir(char* Buffer, size_t Size);

#define MADB_SUPPORTED_CONVERSIONS SQL_CVT_BIGINT | SQL_CVT_BIT | SQL_CVT_CHAR | SQL_CVT_DATE |\
SQL_CVT_DECIMAL | SQL_CVT_DOUBLE | SQL_CVT_FLOAT |\
Expand Down
11 changes: 10 additions & 1 deletion ma_environment.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ extern Client_Charset utf8;
extern MARIADB_CHARSET_INFO* DmUnicodeCs;
extern MARIADB_CHARSET_INFO dummyUtf32le;
Client_Charset SourceAnsiCs= {0, 0}; /* Basically it should be initialized with 0 anyway */
char* DefaultPluginLocation= NULL;
#ifndef _MAX_PATH
# define _MAX_PATH 260
#endif
static char PluginLocationBuf[_MAX_PATH];

MARIADB_CHARSET_INFO * mysql_find_charset_name(const char *name);

Expand Down Expand Up @@ -114,7 +119,11 @@ MADB_Env *MADB_EnvInit()
utf8.cs_info= mariadb_get_charset_by_name("utf8mb4");
GetDefaultLogDir();
GetSourceAnsiCs(&SourceAnsiCs);

/* If we have something in the buffer - then we've already tried to get default location w/out much success */
if (DefaultPluginLocation == NULL && strlen(PluginLocationBuf) == 0)
{
DefaultPluginLocation= MADB_GetDefaultPluginsDir(PluginLocationBuf, sizeof(PluginLocationBuf));
}
cleanup:
#ifdef _WIN32
if (!Env)
Expand Down
2 changes: 1 addition & 1 deletion ma_platform_posix.c
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ BOOL MADB_DSN_PossibleConnect(MADB_Dsn *Dsn)


/* Stub - atm it looks like we don't need to do anything here */
const char* MADB_GetDefaultPluginsDir(MADB_Dbc *Dbc)
char* MADB_GetDefaultPluginsDir(char* Buffer, size_t Size)
{
return NULL;
}
24 changes: 13 additions & 11 deletions ma_platform_win32.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
/* NOTE If you change something in this program, please consider if other platform's version
of the function you are changing, needs to be changed accordingly */

#include <ma_odbc.h>
#include "Shlwapi.h"
#include "ma_odbc.h"
#include <pathcch.h>

extern Client_Charset utf8;
char LogFile[256];
Expand Down Expand Up @@ -295,23 +295,25 @@ BOOL MADB_DirectoryExists(const char *Path)
return (FileAttributes != INVALID_FILE_ATTRIBUTES) && (FileAttributes & FILE_ATTRIBUTE_DIRECTORY);
}

const char* MADB_GetDefaultPluginsDir(MADB_Dbc *Dbc)
char* MADB_GetDefaultPluginsDir(char* Buffer, size_t Size)
{
HMODULE hModule = GetModuleHandle(MADB_DRIVER_NAME);
static char OurLocation[_MAX_PATH];
wchar_t wOurLocation[_MAX_PATH];
const char *PluginsSubDirName= "\\"MADB_DEFAULT_PLUGINS_SUBDIR;
HRESULT hr;

memset(OurLocation, 0, sizeof(OurLocation));
GetModuleFileName(hModule, OurLocation, _MAX_PATH);
PathRemoveFileSpec(OurLocation);
memset(Buffer, 0, Size);
GetModuleFileNameW(hModule, wOurLocation, _MAX_PATH);
hr= PathCchRemoveFileSpec(wOurLocation, _MAX_PATH);

if (strlen(OurLocation) < _MAX_PATH - strlen(PluginsSubDirName))
WideCharToMultiByte(GetACP(), 0, wOurLocation, -1, Buffer, Size, NULL, NULL);
if (strlen(Buffer) < Size - strlen(PluginsSubDirName))
{
strcpy(OurLocation + strlen(OurLocation), PluginsSubDirName);
strcpy(Buffer + strlen(Buffer), PluginsSubDirName);

if (MADB_DirectoryExists(OurLocation) != FALSE)
if (MADB_DirectoryExists(Buffer) != FALSE)
{
return OurLocation;
return Buffer;
}
}
return NULL;
Expand Down
5 changes: 4 additions & 1 deletion ma_platform_win32.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@

#define _ma_platform_x_h_

#define WIN32_LEAN_AND_MEAN
#ifndef WIN32_LEAN_AND_MEAN
# define WIN32_LEAN_AND_MEAN
#endif // !WIN32_LEAN_AND_MEAN

#define _WINSOCKAPI_
#define DONT_DEFINE_VOID
#define HAVE_UNICODE
Expand Down
4 changes: 2 additions & 2 deletions ma_result.c
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,8 @@ SQLRETURN MADB_StmtMoreResults(MADB_Stmt *Stmt)
mysql_stmt_data_seek(Stmt->stmt, 0);
}
}
UNLOCK_MARIADB(Stmt->Connection);
}
UNLOCK_MARIADB(Stmt->Connection);

return ret;
}
Expand All @@ -232,7 +232,7 @@ SQLRETURN MADB_StmtMoreResults(MADB_Stmt *Stmt)
/* {{{ MADB_RecordsToFetch */
SQLULEN MADB_RowsToFetch(MADB_Cursor *Cursor, SQLULEN ArraySize, unsigned long long RowsInResultst)
{
SQLLEN Position= Cursor->Position >= 0 ? Cursor->Position : 0;
SQLULEN Position= Cursor->Position >= 0 ? Cursor->Position : 0;
SQLULEN result= ArraySize;

Cursor->RowsetSize= ArraySize;
Expand Down
2 changes: 1 addition & 1 deletion wininstall/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ ELSE()
ENDIF()

ADD_EXECUTABLE(change_dsns_driver change_dsns_driver.c ${CMAKE_SOURCE_DIR}/ma_dsn.c ${CMAKE_SOURCE_DIR}/ma_platform_win32.c ${CMAKE_SOURCE_DIR}/ma_common.c)
TARGET_LINK_LIBRARIES(change_dsns_driver ${ODBC_LIBS} ${ODBC_INSTLIBS} legacy_stdio_definitions Shlwapi)
TARGET_LINK_LIBRARIES(change_dsns_driver ${ODBC_LIBS} ${ODBC_INSTLIBS} legacy_stdio_definitions Shlwapi Pathcch)

CONFIGURE_FILE(${CMAKE_SOURCE_DIR}/wininstall/mariadb_odbc.xml.in
${CMAKE_BINARY_DIR}/wininstall/mariadb_odbc.xml)
Expand Down

0 comments on commit 2b420d7

Please sign in to comment.