Skip to content
This repository was archived by the owner on Jun 7, 2021. It is now read-only.

Commit 85d1596

Browse files
author
selvaganesang
committed
[TRAFODION-2514] Obscure cores seen in Trafodion while running jenkins tests with RH7
The global object gv_sb_thread_table was not constructed in a thread safe manner. There was buffer overrun while creating explain fragment involving hive tables.
1 parent 63ab728 commit 85d1596

2 files changed

Lines changed: 39 additions & 26 deletions

File tree

‎core/sqf/src/seabed/src/threadl.cpp‎

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -149,12 +149,8 @@ class SB_Thread_Table_Entry_Mgr : public SB_Table_Entry_Mgr<SB_Thread_Ctx_Type>
149149
static void sb_thread_ctx_key_dtor(void *pp_ctx);
150150
static void sb_thread_name_key_dtor(void *pp_name);
151151

152-
SB_Thread_Table_Entry_Mgr gv_sb_thread_table_entry_mgr;
153-
SB_Ts_Table_Mgr<SB_Thread_Ctx_Type> gv_sb_thread_table("tablemgr-THREAD-MGR",
154-
SB_Table_Mgr_Alloc::ALLOC_FAST,
155-
SB_Table_Mgr_Alloc::ALLOC_ENTRY_DYN,
156-
&gv_sb_thread_table_entry_mgr,
157-
10, 10); // cap-init, cap-inc
152+
SB_Ts_Table_Mgr<SB_Thread_Ctx_Type> *gv_sb_thread_table = NULL;
153+
SB_Thread_Table_Entry_Mgr *gv_sb_thread_table_entry_mgr = NULL;
158154
static int gv_sb_thread_ctx_tls_inx =
159155
SB_create_tls_key(sb_thread_ctx_key_dtor,
160156
"thread-ctx");
@@ -166,6 +162,24 @@ static int gv_sb_thread_name_tls_inx =
166162
SB_Thread_Lock_Stats gv_sb_lock_stats;
167163
#endif
168164

165+
SB_Ts_Table_Mgr<SB_Thread_Ctx_Type> *getGlobalTsTableMgr() {
166+
if (gv_sb_thread_table != NULL)
167+
return gv_sb_thread_table;
168+
SB_util_short_lock();
169+
if (gv_sb_thread_table != NULL) {
170+
SB_util_short_unlock();
171+
return gv_sb_thread_table;
172+
}
173+
gv_sb_thread_table_entry_mgr = new SB_Thread_Table_Entry_Mgr();
174+
gv_sb_thread_table = new SB_Ts_Table_Mgr<SB_Thread_Ctx_Type> ("tablemgr-THREAD-MGR",
175+
SB_Table_Mgr_Alloc::ALLOC_FAST,
176+
SB_Table_Mgr_Alloc::ALLOC_ENTRY_DYN,
177+
gv_sb_thread_table_entry_mgr,
178+
10, 10); // cap-init, cap-inc
179+
SB_util_short_unlock();
180+
return gv_sb_thread_table;
181+
}
182+
169183

170184
typedef struct Sthr_Disp {
171185
int iv_ctx_id;
@@ -189,7 +203,7 @@ static void sb_thread_ctx_key_dtor(void *pp_ctx) {
189203
trace_where_printf(WHERE,
190204
"thread exiting ctx-id=%d, name=%s\n",
191205
lp_ctx->iv_ctx_id, lp_ctx->ia_thread_name);
192-
gv_sb_thread_table.free_entry(lp_ctx->iv_ctx_id);
206+
getGlobalTsTableMgr()->free_entry(lp_ctx->iv_ctx_id);
193207
}
194208

195209
static void sb_thread_name_key_dtor(void *pp_name) {
@@ -232,7 +246,7 @@ static void *sb_thread_sthr_disp(void *pp_arg) {
232246
lp_arg = lp_disp->ip_arg;
233247
lp_name = lp_disp->ip_name;
234248
lv_ctx_id = lp_disp->iv_ctx_id;
235-
lp_ctx = gv_sb_thread_table.get_entry(lv_ctx_id);
249+
lp_ctx = getGlobalTsTableMgr()->get_entry(lv_ctx_id);
236250
lp_ctx->ip_fun = lp_fun;
237251
lp_ctx->iv_self = pthread_self();
238252
lp_ctx->iv_tid = gettid();
@@ -261,8 +275,8 @@ void SB_thread_main() {
261275
SB_Thread_Ctx_Type *lp_ctx;
262276
int lv_ctx_id;
263277

264-
lv_ctx_id = static_cast<int>(gv_sb_thread_table.alloc_entry());
265-
lp_ctx = gv_sb_thread_table.get_entry(lv_ctx_id);
278+
lv_ctx_id = static_cast<int>(getGlobalTsTableMgr()->alloc_entry());
279+
lp_ctx = getGlobalTsTableMgr()->get_entry(lv_ctx_id);
266280
lp_ctx->iv_ctx_id = lv_ctx_id;
267281
strcpy(lp_ctx->ia_thread_name, "main");
268282
lp_ctx->iv_self = pthread_self();
@@ -282,9 +296,9 @@ SB_Thread::Sthr::Id_Ptr SB_Thread::Sthr::create(char *pp_name,
282296
lp_disp->ip_arg = pp_arg;
283297
lp_disp->ip_name = pp_name;
284298

285-
lp_disp->iv_ctx_id = static_cast<int>(gv_sb_thread_table.alloc_entry());
299+
lp_disp->iv_ctx_id = static_cast<int>(getGlobalTsTableMgr()->alloc_entry());
286300
SB_Thread_Ctx_Type *lp_ctx =
287-
gv_sb_thread_table.get_entry(lp_disp->iv_ctx_id);
301+
getGlobalTsTableMgr()->get_entry(lp_disp->iv_ctx_id);
288302
lp_ctx->iv_ctx_id = lp_disp->iv_ctx_id;
289303
lp_ctx->ia_thread_name[sizeof(lp_ctx->ia_thread_name)-1] = '\0';
290304
strncpy(lp_ctx->ia_thread_name,

‎core/sql/generator/GenExplain.cpp‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -954,60 +954,60 @@ HbaseAccess::addSpecificExplainInfo(ExplainTupleMaster *explainTuple,
954954
description += "small_scanner: " ;
955955
description += "ON " ;
956956
}
957-
958-
char buf[20];
957+
size_t BUFFER_SIZE=512;
958+
char buf[BUFFER_SIZE];
959959

960960
if ((((ComTdbHbaseAccess *)tdb)->getHbasePerfAttributes()->dopParallelScanner())>0.0) {
961961
description += "parallel_scanner: " ;
962-
sprintf(buf, "%g ", ((ComTdbHbaseAccess *)tdb)->getHbasePerfAttributes()->dopParallelScanner());
962+
snprintf(buf, BUFFER_SIZE, "%g ", ((ComTdbHbaseAccess *)tdb)->getHbasePerfAttributes()->dopParallelScanner());
963963
description += buf;
964964
}
965965

966966
if ( getProbes().getValue() > 0.0 ) {
967967
description += "probes: "; // total number of probes
968-
sprintf(buf, "%g ", getProbes().getValue());
968+
snprintf(buf, BUFFER_SIZE, "%g ", getProbes().getValue());
969969
description += buf;
970970
}
971971

972972
if ( getSuccessfulProbes().getValue() > 0.0 ) {
973973
description += "successful_probes: "; // # of probes returning data
974-
sprintf(buf, "%g ", getSuccessfulProbes().getValue());
974+
snprintf(buf, BUFFER_SIZE, "%g ", getSuccessfulProbes().getValue());
975975
description += buf;
976976
}
977977

978978
if ( getUniqueProbes().getValue() > 0.0 ) {
979979
description += "unique_probes: "; // # of probes returning 1 row
980-
sprintf(buf, "%g ", getUniqueProbes().getValue());
980+
snprintf(buf, BUFFER_SIZE, "%g ", getUniqueProbes().getValue());
981981
description += buf;
982982
}
983983

984984
if ( getDuplicatedSuccProbes().getValue() > 0.0 ) {
985985
description += "duplicated_succ_probes: "; // # of succ probes returning
986-
sprintf(buf, "%g ", getDuplicatedSuccProbes().getValue()); // more than 1 row
986+
snprintf(buf, BUFFER_SIZE, "%g ", getDuplicatedSuccProbes().getValue()); // more than 1 row
987987
description += buf;
988988
}
989989

990990
if ( getEstRowsAccessed().getValue() ) {
991991
description += "rows_accessed: "; // # rows accessed
992-
sprintf(buf, "%g ", getEstRowsAccessed().getValue());
992+
snprintf(buf, BUFFER_SIZE, "%g ", getEstRowsAccessed().getValue());
993993
description += buf;
994994
}
995995
if (((ComTdbHbaseAccess *)tdb)->getHbaseSnapshotScanAttributes()->getUseSnapshotScan())
996996
{
997997
description += "use_snapshot_scan: ";
998-
sprintf(buf, "%s ", "TRUE" );
998+
snprintf(buf, BUFFER_SIZE, "%s ", "TRUE" );
999999
description += buf;
10001000

10011001
description += "full_table_name: ";
1002-
sprintf(buf, "%s ", ((ComTdbHbaseAccess *)tdb)->getTableName());
1002+
snprintf(buf, BUFFER_SIZE, "%s ", ((ComTdbHbaseAccess *)tdb)->getTableName());
10031003
description += buf;
10041004

10051005
description += "snapshot_name: ";
1006-
sprintf(buf, "%s ", ((ComTdbHbaseAccess *)tdb)->getHbaseSnapshotScanAttributes()->getSnapshotName());
1006+
snprintf(buf, BUFFER_SIZE, "%s ", ((ComTdbHbaseAccess *)tdb)->getHbaseSnapshotScanAttributes()->getSnapshotName());
10071007
description += buf;
10081008

10091009
description += "snapshot_temp_location: ";
1010-
sprintf(buf, "%s ", ((ComTdbHbaseAccess *)tdb)->getHbaseSnapshotScanAttributes()->getSnapScanTmpLocation());
1010+
snprintf(buf, BUFFER_SIZE, "%s ", ((ComTdbHbaseAccess *)tdb)->getHbaseSnapshotScanAttributes()->getSnapScanTmpLocation());
10111011
description += buf;
10121012

10131013
}
@@ -1031,9 +1031,8 @@ HbaseAccess::addSpecificExplainInfo(ExplainTupleMaster *explainTuple,
10311031
/*
10321032
// now get columns_retrieved
10331033
description += "columns_retrieved: ";
1034-
char buf[27];
10351034
//sprintf(buf, "%d ", retrievedCols().entries());
1036-
sprintf(buf, "%d ", getIndexDesc()->getIndexColumns().entries());
1035+
snprintf(buf, BUFFER_SIZE, "%d ", getIndexDesc()->getIndexColumns().entries());
10371036
description += buf;
10381037
*/
10391038

0 commit comments

Comments
 (0)