Skip to content

Fix SIGSEGV in CFileNameManager caused by stale share data pointer across test suites #2534

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Copilot wants to merge 14 commits into from
Closed

Fix SIGSEGV in CFileNameManager caused by stale share data pointer across test suites #2534

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
f8e9a51
SetUpShareDataに言語環境の初期化を含める
berryzplus Jul 18, 2026
4a9c4b9
TrayWndテストの実装を見直す
berryzplus Jul 18, 2026
c38801c
TrayWndのメッセージ配送をテストする(1/2)
berryzplus Jul 18, 2026
9c16c18
メニュークリックをテストできるようにする
berryzplus Jul 18, 2026
64baadd
トレイメニュークリックのテストを追加する
berryzplus Jul 18, 2026
cc11a10
WM_HELPハンドラの定義を修正する
berryzplus Jul 18, 2026
c63c151
MYWM_HTMLHELPハンドラの定義を修正する
berryzplus Jul 18, 2026
c35dede
SonarQube指摘対応 Merge this "if" statement with the enclosing one.
berryzplus Jul 18, 2026
6ef32b7
SonarQube指摘対応 Make the type of this variable a pointer-to-const.
berryzplus Jul 18, 2026
043007d
SonarQube指摘対応 Use the init-statement
berryzplus Jul 18, 2026
8f800ba
Initial plan
Copilot Jul 19, 2026
108bd8c
Merge feature/improve_traywnd_test with CFileNameManager fix
Copilot Jul 19, 2026
844aa27
CFileNameManagerのm_pShareDataキャッシュを排除して共有データ直接参照に変更する
Copilot Jul 19, 2026
239e5d7
Fix typo in comment: 表示しする -> 表示する
Copilot Jul 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 18 additions & 24 deletions sakura_core/_main/CControlTray.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,21 +96,21 @@ void CControlTray::DoGrep()
m_cDlgGrep.m_strText = m_pShareData->m_sSearchKeywords.m_aSearchKeys[0];
}
if( 0 < m_pShareData->m_sSearchKeywords.m_aGrepFiles.size() ){
wcscpy( m_cDlgGrep.m_szFile, m_pShareData->m_sSearchKeywords.m_aGrepFiles[0] ); /* 検索ファイル */
m_cDlgGrep.m_szFile = m_pShareData->m_sSearchKeywords.m_aGrepFiles[0]; /* 検索ファイル */
}
if( 0 < m_pShareData->m_sSearchKeywords.m_aGrepFolders.size() ){
wcscpy( m_cDlgGrep.m_szFolder, m_pShareData->m_sSearchKeywords.m_aGrepFolders[0] ); /* 検索フォルダー */
m_cDlgGrep.m_szFolder = m_pShareData->m_sSearchKeywords.m_aGrepFolders[0]; /* 検索フォルダー */
}
if (0 < m_pShareData->m_sSearchKeywords.m_aExcludeFiles.size()) {
wcscpy(m_cDlgGrep.m_szExcludeFile, m_pShareData->m_sSearchKeywords.m_aExcludeFiles[0]); /* 除外ファイル */
m_cDlgGrep.m_szExcludeFile = m_pShareData->m_sSearchKeywords.m_aExcludeFiles[0]; /* 除外ファイル */
}
if (0 < m_pShareData->m_sSearchKeywords.m_aExcludeFolders.size()) {
wcscpy(m_cDlgGrep.m_szExcludeFolder, m_pShareData->m_sSearchKeywords.m_aExcludeFolders[0]); /* 除外フォルダー */
m_cDlgGrep.m_szExcludeFolder = m_pShareData->m_sSearchKeywords.m_aExcludeFolders[0]; /* 除外フォルダー */
}

/* Grepダイアログの表示 */
int nRet = m_cDlgGrep.DoModal( m_hInstance, nullptr, L"" );
if( !nRet || GetTrayHwnd() == nullptr ){
if (const auto nRet = m_cDlgGrep.DoModal(m_hInstance, nullptr, L"");
!nRet || GetTrayHwnd() == nullptr ){
return;
}
m_nCurSearchKeySequence = GetDllShareData().m_Common.m_sSearch.m_nSearchKeySequence;
Expand Down Expand Up @@ -384,7 +384,6 @@ LRESULT CControlTray::DispatchEvent(

int nId;
HWND hwndWork;
LPHELPINFO lphi;

int nRowNum;
EditNode* pEditNodeArr;
Expand Down Expand Up @@ -461,7 +460,7 @@ LRESULT CControlTray::DispatchEvent(
case MYWM_HTMLHELP:
{
auto &sWorkBuffer = m_pShareData->m_sWorkBuffer;
WCHAR* pWork = sWorkBuffer.GetWorkBuffer<WCHAR>();
const auto pWork = sWorkBuffer.GetWorkBuffer<WCHAR>();

// pszHelpFile取得
const WCHAR* pszHelpFile = pWork;
Expand All @@ -470,6 +469,8 @@ LRESULT CControlTray::DispatchEvent(
// pszKeywords取得
const WCHAR* pszKeywords = &pWork[cchHelpFile + 1];

if (!*pszHelpFile) return 0L;

// Jul. 6, 2001 genta HtmlHelpの呼び出し方法変更
hwndHtmlHelp = OpenHtmlHelp(
nullptr,
Expand Down Expand Up @@ -564,13 +565,8 @@ LRESULT CControlTray::DispatchEvent(

// case WM_QUERYENDSESSION:
case WM_HELP:
lphi = (LPHELPINFO) lParam;
switch( lphi->iContextType ){
case HELPINFO_MENUITEM:
if (const auto lphi = (LPHELPINFO) lParam; lphi && HELPINFO_MENUITEM == lphi->iContextType) {
MyWinHelp( hwnd, HELP_CONTEXT, FuncID_To_HelpContextID( (EFunctionCode)lphi->iCtrlId ) );
break;
default:
break;
}
return TRUE;
case WM_COMMAND:
Expand Down Expand Up @@ -1442,24 +1438,23 @@ void CControlTray::TerminateApplication(
HWND hWndFrom //!< [in] 呼び出し元のウィンドウハンドル
)
{
DLLSHAREDATA* pShareData = &GetDllShareData(); /* 共有データ構造体のアドレスを返す */
const auto pShareData = &GetDllShareData(); /* 共有データ構造体のアドレスを返す */

/* 現在の編集ウィンドウの数を調べる */
if( pShareData->m_Common.m_sGeneral.m_bExitConfirm ){ //終了時の確認
if( 0 < CAppNodeGroupHandle(0).GetEditorWindowsNum() ){
if( IDYES != ::MYMESSAGEBOX(
if (pShareData->m_Common.m_sGeneral.m_bExitConfirm && //終了時の確認
0 < CAppNodeGroupHandle(0).GetEditorWindowsNum() &&
IDYES != ::MessageBoxF(
hWndFrom,
MB_YESNO | MB_APPLMODAL | MB_ICONQUESTION,
GSTR_APPNAME,
LS(STR_TRAY_EXITALL)
) ){
return;
}
}
}

/* 「すべてのウィンドウを閉じる」要求 */ //Oct. 7, 2000 jepro 「編集ウィンドウの全終了」という説明を左記のように変更
BOOL bCheckConfirm = (pShareData->m_Common.m_sGeneral.m_bExitConfirm)? FALSE: TRUE; // 2006.12.25 ryoji 終了確認済みならそれ以上は確認しない
if( CloseAllEditor( bCheckConfirm, hWndFrom, TRUE, 0 ) ){ // 2006.12.25, 2007.02.13 ryoji 引数追加
if (const auto bCheckConfirm = pShareData->m_Common.m_sGeneral.m_bExitConfirm; // 2006.12.25 ryoji 終了確認済みならそれ以上は確認しない
CloseAllEditor(bCheckConfirm, hWndFrom, TRUE, 0)) { // 2006.12.25, 2007.02.13 ryoji 引数追加
::PostMessageAny( pShareData->m_sHandles.m_hwndTray, WM_CLOSE, 0, 0 );
}
return;
Expand Down Expand Up @@ -1506,7 +1501,6 @@ int CControlTray::CreatePopUpMenu_L( void )
WCHAR szMenu[100 + MAX_PATH * 2]; // Jan. 19, 2001 genta
POINT po;
RECT rc;
EditInfo* pfi;

//本当はセマフォにしないとだめ
if( m_bUseTrayMenu ) return -1;
Expand Down Expand Up @@ -1568,7 +1562,7 @@ int CControlTray::CreatePopUpMenu_L( void )
if( IsSakuraMainWindow( m_pShareData->m_sNodes.m_pEditArr[i].GetHwnd() ) ){
/* トレイからエディタへの編集ファイル名要求通知 */
::SendMessage( m_pShareData->m_sNodes.m_pEditArr[i].GetHwnd(), MYWM_GETFILEINFO, 0, 0 );
pfi = (EditInfo*)&m_pShareData->m_sWorkBuffer.m_EditInfo_MYWM_GETFILEINFO;
const auto pfi = &m_pShareData->m_sWorkBuffer.m_EditInfo_MYWM_GETFILEINFO;

// メニューラベル。1からアクセスキーを振る
CFileNameManager::getInstance()->GetMenuFullLabel_WinList( szMenu, int(std::size(szMenu)), pfi, m_pShareData->m_sNodes.m_pEditArr[i].m_nId, i, dcFont.GetHDC() );
Expand Down
1 change: 1 addition & 0 deletions sakura_core/_main/CControlTray.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ class CControlTray
bool OnAddTypeSetting(size_t index);
bool OnDelTypeSetting(size_t index);

public: // テストできないのでアクセス権変更
/*
|| メンバ変数
*/
Expand Down
15 changes: 8 additions & 7 deletions sakura_core/env/CFileNameManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ LPWSTR CFileNameManager::GetTransformFileNameFast( LPCWSTR pszSrc, LPWSTR pszDes
}

int nPxWidth = -1;
if( m_pShareData->m_Common.m_sFileName.m_bTransformShortPath && cchMaxWidth != -1 ){
if( GetDllShareData().m_Common.m_sFileName.m_bTransformShortPath && cchMaxWidth != -1 ){
if( cchMaxWidth == 0 ){
cchMaxWidth = m_pShareData->m_Common.m_sFileName.m_nTransformShortMaxWidth;
cchMaxWidth = GetDllShareData().m_Common.m_sFileName.m_nTransformShortMaxWidth;
}
CTextWidthCalc calc(hDC);
nPxWidth = calc.GetTextWidth(L"x") * cchMaxWidth;
Expand All @@ -54,13 +54,13 @@ LPWSTR CFileNameManager::GetTransformFileNameFast( LPCWSTR pszSrc, LPWSTR pszDes
if( 0 < m_nTransformFileNameCount ){
GetFilePathFormat( pszSrc, pszDest, nDestLen,
m_szTransformFileNameFromExp[0],
m_pShareData->m_Common.m_sFileName.m_szTransformFileNameTo[m_nTransformFileNameOrgId[0]]
GetDllShareData().m_Common.m_sFileName.m_szTransformFileNameTo[m_nTransformFileNameOrgId[0]]
);
for( i = 1; i < m_nTransformFileNameCount; i++ ){
wcscpy( szBuf, pszDest );
GetFilePathFormat( szBuf, pszDest, nDestLen,
m_szTransformFileNameFromExp[i],
m_pShareData->m_Common.m_sFileName.m_szTransformFileNameTo[m_nTransformFileNameOrgId[i]] );
GetDllShareData().m_Common.m_sFileName.m_szTransformFileNameTo[m_nTransformFileNameOrgId[i]] );
}
if( nPxWidth != -1 ){
wcscpy( szBuf, pszDest );
Expand All @@ -84,9 +84,10 @@ LPWSTR CFileNameManager::GetTransformFileNameFast( LPCWSTR pszSrc, LPWSTR pszDes
int CFileNameManager::TransformFileName_MakeCache( void ){
int i;
int nCount = 0;
for( i = 0; i < m_pShareData->m_Common.m_sFileName.m_nTransformFileNameArrNum; i++ ){
if( L'\0' != m_pShareData->m_Common.m_sFileName.m_szTransformFileNameFrom[i][0] ){
if( ExpandMetaToFolder( m_pShareData->m_Common.m_sFileName.m_szTransformFileNameFrom[i],
const auto& sFileName = GetDllShareData().m_Common.m_sFileName;
for( i = 0; i < sFileName.m_nTransformFileNameArrNum; i++ ){
if( L'\0' != sFileName.m_szTransformFileNameFrom[i][0] ){
if( ExpandMetaToFolder( sFileName.m_szTransformFileNameFrom[i],
m_szTransformFileNameFromExp[nCount], _MAX_PATH ) ){
// m_szTransformFileNameToとm_szTransformFileNameFromExpの番号がずれることがあるので記録しておく
m_nTransformFileNameOrgId[nCount] = i;
Expand Down
3 changes: 0 additions & 3 deletions sakura_core/env/CFileNameManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ class CFileNameManager : public TSingleton<CFileNameManager>{
friend class TSingleton<CFileNameManager>;
CFileNameManager()

@berryzplus berryzplus Jul 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

「このクラスがシングルトンであること」が誤り。

前提

シングルトンクラスは、状態を持ってはならない。

いつ生成しても不変なオブジェクトを何度も生成しないための仕組みがシングルトン。

2つ以上の状態を持つオブジェクトをシングルトンにしてはならない。
TSingleton テンプレート利用クラスのほとんどがこれにあたるので、本当は直さないといけない。

この修正の趣旨

状態変数 m_pShareData の代わりに GetDllShareDataPtr() を使う。

ダメ出しポイント

もともと、状態変数 m_pShareData は絶対 NULL にならない。
したがって、m_pShareData が NULL だった場合のガードは存在しない。

GetDllShareData() は NULL を返し得る関数 として定義している。
この関数の戻り値は deref する前にNULLチェックする必要がある。

つまり、こうなる。

仕様上NULLにならないからチェックしてないコード。
　↓
仕様上NULLになり得るけど移行した結果チェックが漏れたコード。

{
m_pShareData = &GetDllShareData();
m_nTransformFileNameCount = -1;
}

Expand Down Expand Up @@ -60,8 +59,6 @@ class CFileNameManager : public TSingleton<CFileNameManager>{
static WCHAR GetAccessKeyByIndex(int index, bool bZeroOrigin);

private:
DLLSHAREDATA* m_pShareData;

// ファイル名簡易表示用キャッシュ
int m_nTransformFileNameCount; // 有効数
WCHAR m_szTransformFileNameFromExp[MAX_TRANSFORM_FILENAME][_MAX_PATH];
Expand Down
3 changes: 3 additions & 0 deletions src/test/cpp/tests1/env/ShareDataTestSuite.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ namespace env {
*/
/* static */ void ShareDataTestSuite::SetUpShareData()
{
// 言語環境を初期化する
CSelectLang::InitializeLanguageEnvironment();

pcShareData = std::make_unique<CShareData>();

EXPECT_THAT(pcShareData->InitShareData(), IsTrue());
Expand Down
Loading