From: Andrew Eikum Subject: [PATCH 2/3] user32: Apply reference counting to all uses of POPUPMENU Message-Id: <20140912154550.GG4782@foghorn.codeweavers.com> Date: Fri, 12 Sep 2014 10:45:50 -0500 We could also take the simpler approach of only incrementing the refcount when we enter user code, though this is more consistent. Adding reference counting this early adds a bit of churn. We have to add a bunch of reference releases, only to remove them once we no longer hold the extra references in future patches. --- dlls/user32/menu.c | 361 +++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 294 insertions(+), 67 deletions(-) diff --git a/dlls/user32/menu.c b/dlls/user32/menu.c index ae366cb..969c441 100644 --- a/dlls/user32/menu.c +++ b/dlls/user32/menu.c @@ -301,8 +301,15 @@ static POPUPMENU *MENU_GetMenu(HMENU hMenu) WARN( "other process menu %p?\n", hMenu); return NULL; } - if (menu) release_user_handle_ptr( menu ); /* FIXME! */ - else WARN("invalid menu handle=%p\n", hMenu); + + if (menu) + { + InterlockedIncrement(&menu->refcount); + release_user_handle_ptr( menu ); /* FIXME! */ + } + else + WARN("invalid menu handle=%p\n", hMenu); + return menu; } @@ -445,6 +452,7 @@ static HMENU MENU_CopySysPopup(BOOL mdi) miteminfo.hbmpItem = HBMMENU_POPUP_MINIMIZE; SetMenuItemInfoW( hMenu, SC_MINIMIZE, FALSE, &miteminfo); SetMenuDefaultItem(hMenu, SC_CLOSE, FALSE); + MENU_ReleaseMenu(menu); } else ERR("Unable to load default system menu\n" ); @@ -494,7 +502,11 @@ static HMENU MENU_GetSysMenu( HWND hWnd, HMENU hPopupMenu ) menu->items[0].fType = MF_SYSMENU | MF_POPUP; menu->items[0].fState = 0; - if ((menu = MENU_GetMenu(hPopupMenu))) menu->wFlags |= MF_SYSMENU; + MENU_ReleaseMenu(menu); + if ((menu = MENU_GetMenu(hPopupMenu))) { + menu->wFlags |= MF_SYSMENU; + MENU_ReleaseMenu(menu); + } TRACE("hMenu=%p (hPopup %p)\n", hMenu, hPopupMenu ); return hMenu; @@ -550,14 +562,20 @@ static UINT MENU_GetStartOfNextColumn( return NO_SELECTED_ITEM; i = menu->FocusedItem + 1; - if( i == NO_SELECTED_ITEM ) + if( i == NO_SELECTED_ITEM ) { + MENU_ReleaseMenu(menu); return i; + } for( ; i < menu->nItems; ++i ) { - if (menu->items[i].fType & (MF_MENUBREAK | MF_MENUBARBREAK)) + if (menu->items[i].fType & (MF_MENUBREAK | MF_MENUBARBREAK)) { + MENU_ReleaseMenu(menu); return i; + } } + MENU_ReleaseMenu(menu); + return NO_SELECTED_ITEM; } @@ -578,8 +596,10 @@ static UINT MENU_GetStartOfPrevColumn( if( !menu ) return NO_SELECTED_ITEM; - if( menu->FocusedItem == 0 || menu->FocusedItem == NO_SELECTED_ITEM ) + if( menu->FocusedItem == 0 || menu->FocusedItem == NO_SELECTED_ITEM ) { + MENU_ReleaseMenu(menu); return NO_SELECTED_ITEM; + } /* Find the start of the column */ @@ -587,14 +607,17 @@ static UINT MENU_GetStartOfPrevColumn( !(menu->items[i].fType & (MF_MENUBREAK | MF_MENUBARBREAK)); --i); /* empty */ - if(i == 0) + if(i == 0){ + MENU_ReleaseMenu(menu); return NO_SELECTED_ITEM; + } for(--i; i != 0; --i) { if (menu->items[i].fType & (MF_MENUBREAK | MF_MENUBARBREAK)) break; } + MENU_ReleaseMenu(menu); TRACE("ret %d.\n", i ); return i; @@ -618,8 +641,13 @@ static MENUITEM *MENU_FindItem( HMENU *hmenu, UINT *nPos, UINT wFlags ) if ((*hmenu == (HMENU)0xffff) || (!(menu = MENU_GetMenu(*hmenu)))) return NULL; if (wFlags & MF_BYPOSITION) { - if (*nPos >= menu->nItems) return NULL; - return &menu->items[*nPos]; + MENUITEM *ret; + if (*nPos >= menu->nItems) + ret = NULL; + else + ret = &menu->items[*nPos]; + MENU_ReleaseMenu(menu); + return ret; } else { @@ -633,6 +661,7 @@ static MENUITEM *MENU_FindItem( HMENU *hmenu, UINT *nPos, UINT wFlags ) if (subitem) { *hmenu = hsubmenu; + MENU_ReleaseMenu(menu); return subitem; } else if (item->wID == *nPos) @@ -645,6 +674,7 @@ static MENUITEM *MENU_FindItem( HMENU *hmenu, UINT *nPos, UINT wFlags ) else if (item->wID == *nPos) { *nPos = i; + MENU_ReleaseMenu(menu); return item; } } @@ -653,6 +683,8 @@ static MENUITEM *MENU_FindItem( HMENU *hmenu, UINT *nPos, UINT wFlags ) if (fallback) *nPos = fallback_pos; + MENU_ReleaseMenu(menu); + return fallback; } @@ -675,6 +707,7 @@ static UINT MENU_FindSubMenu( HMENU *hmenu, HMENU hSubTarget ) for (i = 0; i < menu->nItems; i++, item++) { if(!(item->fType & MF_POPUP)) continue; if (MENU_GetHandle(item->submenu) == hSubTarget) { + MENU_ReleaseMenu(menu); return i; } else { @@ -682,10 +715,12 @@ static UINT MENU_FindSubMenu( HMENU *hmenu, HMENU hSubTarget ) UINT pos = MENU_FindSubMenu( &hsubmenu, hSubTarget ); if (pos != NO_SELECTED_ITEM) { *hmenu = hsubmenu; + MENU_ReleaseMenu(menu); return pos; } } } + MENU_ReleaseMenu(menu); return NO_SELECTED_ITEM; } @@ -788,12 +823,16 @@ static UINT MENU_FindItemByKey( HWND hwndOwner, HMENU hmenu, if (!p && cjk) p = strchrW (q, '\036'); /* Japanese Win16 */ } while (p != NULL && p [1] == '&'); - if (p && (toupperW(p[1]) == toupperW(key))) return i; + if (p && (toupperW(p[1]) == toupperW(key))) { + MENU_ReleaseMenu(menu); + return i; + } } } } menuchar = SendMessageW( hwndOwner, WM_MENUCHAR, MAKEWPARAM( key, menu->wFlags ), (LPARAM)hmenu ); + MENU_ReleaseMenu(menu); if (HIWORD(menuchar) == MNC_EXECUTE) return LOWORD(menuchar); if (HIWORD(menuchar) == MNC_CLOSE) return (UINT)(-2); } @@ -1420,6 +1459,7 @@ static void MENU_DrawMenuItem( HWND hwnd, HMENU hmenu, HWND hwndOwner, HDC hdc, { if( !IsIconic(hwnd) ) NC_DrawSysButton( hwnd, hdc, lpitem->fState & (MF_HILITE | MF_MOUSESELECT) ); + MENU_ReleaseMenu(menu); return; } @@ -1452,7 +1492,7 @@ static void MENU_DrawMenuItem( HWND hwnd, HMENU hmenu, HWND hwndOwner, HDC hdc, TRACE("rect=%s\n", wine_dbgstr_rect( &lpitem->rect)); rect = lpitem->rect; - MENU_AdjustMenuItemRect(MENU_GetMenu(hmenu), &rect); + MENU_AdjustMenuItemRect(menu, &rect); if (lpitem->fType & MF_OWNERDRAW) { @@ -1488,10 +1528,14 @@ static void MENU_DrawMenuItem( HWND hwnd, HMENU hmenu, HWND hwndOwner, HDC hdc, if (lpitem->fType & MF_POPUP) draw_popup_arrow( hdc, rect, arrow_bitmap_width, arrow_bitmap_height); + MENU_ReleaseMenu(menu); return; } - if (menuBar && (lpitem->fType & MF_SEPARATOR)) return; + if (menuBar && (lpitem->fType & MF_SEPARATOR)) { + MENU_ReleaseMenu(menu); + return; + } if (lpitem->fState & MF_HILITE) { @@ -1553,6 +1597,7 @@ static void MENU_DrawMenuItem( HWND hwnd, HMENU hmenu, HWND hwndOwner, HDC hdc, } else DrawEdge (hdc, &rc, EDGE_ETCHED, BF_TOP); + MENU_ReleaseMenu(menu); return; } @@ -1735,6 +1780,8 @@ static void MENU_DrawMenuItem( HWND hwnd, HMENU hmenu, HWND hwndOwner, HDC hdc, if (hfontOld) SelectObject (hdc, hfontOld); } + + MENU_ReleaseMenu(menu); } @@ -1788,6 +1835,7 @@ static void MENU_DrawPopupMenu( HWND hwnd, HDC hdc, HMENU hmenu ) /* draw scroll arrows */ if (menu->bScrolling) MENU_DrawScrollArrows(menu, hdc); + MENU_ReleaseMenu(menu); } } else { @@ -1804,11 +1852,9 @@ static void MENU_DrawPopupMenu( HWND hwnd, HDC hdc, HMENU hmenu ) */ UINT MENU_DrawMenuBar( HDC hDC, LPRECT lprect, HWND hwnd ) { - LPPOPUPMENU lppop; HMENU hMenu = GetMenu(hwnd); - lppop = MENU_GetMenu( hMenu ); - if (lppop == NULL || lprect == NULL) + if (!IsMenu(hMenu) || lprect == NULL) { return GetSystemMetrics(SM_CYMENU); } @@ -1835,6 +1881,7 @@ static BOOL MENU_InitPopup( HWND hwndOwner, HMENU hmenu, UINT flags ) if (!IsWindow( hwndOwner )) { SetLastError( ERROR_INVALID_WINDOW_HANDLE ); + MENU_ReleaseMenu(menu); return FALSE; } menu->hwndOwner = hwndOwner; @@ -1847,7 +1894,12 @@ static BOOL MENU_InitPopup( HWND hwndOwner, HMENU hmenu, UINT flags ) WS_POPUP, 0, 0, 0, 0, hwndOwner, 0, (HINSTANCE)GetWindowLongPtrW(hwndOwner, GWLP_HINSTANCE), (LPVOID)hmenu ); - if( !menu->hWnd ) return FALSE; + if( !menu->hWnd ) { + MENU_ReleaseMenu(menu); + return FALSE; + } + + MENU_ReleaseMenu(menu); return TRUE; } @@ -1929,6 +1981,7 @@ static BOOL MENU_ShowPopup( HWND hwndOwner, HMENU hmenu, UINT id, UINT flags, SetWindowPos( menu->hWnd, HWND_TOPMOST, x, y, width, height, SWP_SHOWWINDOW | SWP_NOACTIVATE ); UpdateWindow( menu->hWnd ); + MENU_ReleaseMenu(menu); return TRUE; } @@ -1986,9 +2039,14 @@ static void MENU_SelectItem( HWND hwndOwner, HMENU hmenu, UINT wIndex, TRACE("owner=%p menu=%p index=0x%04x select=0x%04x\n", hwndOwner, hmenu, wIndex, sendMenuSelect); lppop = MENU_GetMenu( hmenu ); - if ((!lppop) || (!lppop->nItems) || (!lppop->hWnd)) return; + if (!lppop) + return; + + if (!lppop->nItems || !lppop->hWnd || lppop->FocusedItem == wIndex) { + MENU_ReleaseMenu(lppop); + return; + } - if (lppop->FocusedItem == wIndex) return; if (lppop->wFlags & MF_POPUP) hdc = GetDC( lppop->hWnd ); else hdc = GetDCEx( lppop->hWnd, 0, DCX_CACHE | DCX_WINDOW); if (!top_popup) { @@ -2036,10 +2094,12 @@ static void MENU_SelectItem( HWND hwndOwner, HMENU hmenu, UINT wIndex, SendMessageW( hwndOwner, WM_MENUSELECT, MAKEWPARAM(pos, ip->fType | ip->fState | (ptm->wFlags & MF_SYSMENU)), (LPARAM)topmenu); + MENU_ReleaseMenu(ptm); } } } ReleaseDC( lppop->hWnd, hdc ); + MENU_ReleaseMenu(lppop); } @@ -2058,18 +2118,30 @@ static void MENU_MoveSelection( HWND hwndOwner, HMENU hmenu, INT offset ) TRACE("hwnd=%p hmenu=%p off=0x%04x\n", hwndOwner, hmenu, offset); menu = MENU_GetMenu( hmenu ); - if ((!menu) || (!menu->items)) return; + if (!menu) return; + + if (!menu->items) { + MENU_ReleaseMenu(menu); + return; + } if ( menu->FocusedItem != NO_SELECTED_ITEM ) { - if( menu->nItems == 1 ) return; else - for (i = menu->FocusedItem + offset ; i >= 0 && i < menu->nItems - ; i += offset) - if (!(menu->items[i].fType & MF_SEPARATOR)) - { - MENU_SelectItem( hwndOwner, hmenu, i, TRUE, 0 ); - return; - } + if( menu->nItems == 1 ) + { + MENU_ReleaseMenu(menu); + return; + } + + for (i = menu->FocusedItem + offset; i >= 0 && i < menu->nItems; i += offset) + { + if (!(menu->items[i].fType & MF_SEPARATOR)) + { + MENU_SelectItem( hwndOwner, hmenu, i, TRUE, 0 ); + MENU_ReleaseMenu(menu); + return; + } + } } for ( i = (offset > 0) ? 0 : menu->nItems - 1; @@ -2077,8 +2149,11 @@ static void MENU_MoveSelection( HWND hwndOwner, HMENU hmenu, INT offset ) if (!(menu->items[i].fType & MF_SEPARATOR)) { MENU_SelectItem( hwndOwner, hmenu, i, TRUE, 0 ); + MENU_ReleaseMenu(menu); return; } + + MENU_ReleaseMenu(menu); } @@ -2104,6 +2179,7 @@ static MENUITEM *MENU_InsertItem( HMENU hMenu, UINT pos, UINT flags ) if (!MENU_FindItem( &hMenu, &pos, flags )) pos = menu->nItems; else { + MENU_ReleaseMenu(menu); if (!(menu = MENU_GetMenu( hMenu ))) return NULL; } @@ -2125,6 +2201,7 @@ static MENUITEM *MENU_InsertItem( HMENU hMenu, UINT pos, UINT flags ) if (!newItems) { WARN("allocation failed\n" ); + MENU_ReleaseMenu(menu); return NULL; } if (menu->nItems > 0) @@ -2139,6 +2216,7 @@ static MENUITEM *MENU_InsertItem( HMENU hMenu, UINT pos, UINT flags ) menu->nItems++; memset( &newItems[pos], 0, sizeof(*newItems) ); menu->Height = 0; /* force size recalculate */ + MENU_ReleaseMenu(menu); return &newItems[pos]; } @@ -2256,11 +2334,21 @@ static HMENU MENU_GetSubPopup( HMENU hmenu ) menu = MENU_GetMenu( hmenu ); - if ((!menu) || (menu->FocusedItem == NO_SELECTED_ITEM)) return 0; + if (!menu) + return 0; + + if (menu->FocusedItem == NO_SELECTED_ITEM) { + MENU_ReleaseMenu(menu); + return 0; + } item = &menu->items[menu->FocusedItem]; - if ((item->fType & MF_POPUP) && (item->fState & MF_MOUSESELECT)) + if ((item->fType & MF_POPUP) && (item->fState & MF_MOUSESELECT)) { + MENU_ReleaseMenu(menu); return MENU_GetHandle(item->submenu); + } + + MENU_ReleaseMenu(menu); return 0; } @@ -2277,7 +2365,10 @@ static void MENU_HideSubPopups( HWND hwndOwner, HMENU hmenu, TRACE("owner=%p hmenu=%p 0x%04x\n", hwndOwner, hmenu, sendMenuSelect); - if (menu && top_popup) + if (!menu) + return; + + if (top_popup) { HMENU hsubmenu; POPUPMENU *submenu; @@ -2291,9 +2382,15 @@ static void MENU_HideSubPopups( HWND hwndOwner, HMENU hmenu, item->fState &= ~MF_MOUSESELECT; submenu = item->submenu; hsubmenu = MENU_GetHandle(item->submenu); - } else return; + } else { + MENU_ReleaseMenu(menu); + return; + } - if (!submenu) return; + if (!submenu) { + MENU_ReleaseMenu(menu); + return; + } MENU_HideSubPopups( hwndOwner, hsubmenu, FALSE, wFlags ); MENU_SelectItem( hwndOwner, hsubmenu, NO_SELECTED_ITEM, sendMenuSelect, 0 ); DestroyWindow( submenu->hWnd ); @@ -2303,6 +2400,8 @@ static void MENU_HideSubPopups( HWND hwndOwner, HMENU hmenu, SendMessageW( hwndOwner, WM_UNINITMENUPOPUP, (WPARAM)hsubmenu, MAKELPARAM(0, IS_SYSTEM_MENU(submenu)) ); } + + MENU_ReleaseMenu(menu); } @@ -2324,11 +2423,16 @@ static HMENU MENU_ShowSubPopup( HWND hwndOwner, HMENU hmenu, if (!(menu = MENU_GetMenu( hmenu ))) return hmenu; - if (menu->FocusedItem == NO_SELECTED_ITEM) return hmenu; + if (menu->FocusedItem == NO_SELECTED_ITEM) { + MENU_ReleaseMenu(menu); + return hmenu; + } item = &menu->items[menu->FocusedItem]; - if (!(item->fType & MF_POPUP) || (item->fState & (MF_GRAYED | MF_DISABLED))) + if (!(item->fType & MF_POPUP) || (item->fState & (MF_GRAYED | MF_DISABLED))) { + MENU_ReleaseMenu(menu); return hmenu; + } /* message must be sent before using item, because nearly everything may be changed by the application ! */ @@ -2411,6 +2515,7 @@ static HMENU MENU_ShowSubPopup( HWND hwndOwner, HMENU hmenu, rect.left, rect.top, rect.right, rect.bottom ); if (selectFirst) MENU_MoveSelection( hwndOwner, MENU_GetHandle(item->submenu), ITEM_NEXT ); + MENU_ReleaseMenu(menu); return MENU_GetHandle(item->submenu); } @@ -2435,7 +2540,11 @@ void MENU_EndMenu( HWND hwnd ) { POPUPMENU *menu; menu = top_popup_hmenu ? MENU_GetMenu( top_popup_hmenu ) : NULL; - if (menu && (hwnd == menu->hWnd || hwnd == menu->hwndOwner)) EndMenu(); + if (!menu) + return; + if (hwnd == menu->hWnd || hwnd == menu->hwndOwner) + EndMenu(); + MENU_ReleaseMenu(menu); } /*********************************************************************** @@ -2467,6 +2576,7 @@ static HMENU MENU_PtMenu( HMENU hMenu, POINT pt ) else if (ht == HTMENU) ret = GetMenu( menu->hWnd ); } + MENU_ReleaseMenu(menu); return ret; } @@ -2487,8 +2597,13 @@ static INT MENU_ExecFocusedItem( MTRACKER* pmt, HMENU hMenu, UINT wFlags ) TRACE("%p hmenu=%p\n", pmt, hMenu); - if (!menu || !menu->nItems || - (menu->FocusedItem == NO_SELECTED_ITEM)) return -1; + if (!menu) + return -1; + + if (!menu->nItems || menu->FocusedItem == NO_SELECTED_ITEM) { + MENU_ReleaseMenu(menu); + return -1; + } item = &menu->items[menu->FocusedItem]; @@ -2517,15 +2632,18 @@ static INT MENU_ExecFocusedItem( MTRACKER* pmt, HMENU hMenu, UINT wFlags ) PostMessageW( pmt->hOwnerWnd, WM_COMMAND, item->wID, 0 ); } } + MENU_ReleaseMenu(menu); return item->wID; } } else { pmt->hCurrentMenu = MENU_ShowSubPopup(pmt->hOwnerWnd, hMenu, TRUE, wFlags); + MENU_ReleaseMenu(menu); return -2; } + MENU_ReleaseMenu(menu); return -1; } @@ -2551,6 +2669,8 @@ static void MENU_SwitchTracking( MTRACKER* pmt, HMENU hPtMenu, UINT id, UINT wFl } else MENU_HideSubPopups( pmt->hOwnerWnd, hPtMenu, FALSE, wFlags ); MENU_SelectItem( pmt->hOwnerWnd, hPtMenu, id, TRUE, 0 ); + MENU_ReleaseMenu(ptmenu); + MENU_ReleaseMenu(topmenu); } @@ -2585,8 +2705,10 @@ static BOOL MENU_ButtonDown( MTRACKER* pmt, HMENU hPtMenu, UINT wFlags ) pmt->hCurrentMenu = MENU_ShowSubPopup( pmt->hOwnerWnd, hPtMenu, FALSE, wFlags ); } + MENU_ReleaseMenu(ptmenu); return TRUE; } + MENU_ReleaseMenu(ptmenu); /* Else the click was on the menu bar, finish the tracking */ } return FALSE; @@ -2622,18 +2744,25 @@ static INT MENU_ButtonUp( MTRACKER* pmt, HMENU hPtMenu, UINT wFlags) if( !(item->fType & MF_POPUP) ) { INT executedMenuId = MENU_ExecFocusedItem( pmt, hPtMenu, wFlags); - if (executedMenuId == -1 || executedMenuId == -2) return -1; + if (executedMenuId == -1 || executedMenuId == -2) { + MENU_ReleaseMenu(ptmenu); + return -1; + } + MENU_ReleaseMenu(ptmenu); return executedMenuId; } /* If we are dealing with the menu bar */ /* and this is a click on an already "popped" item: */ /* Stop the menu tracking and close the opened submenus */ - if((pmt->hTopMenu == hPtMenu) && ptmenu->bTimeToHide) + if((pmt->hTopMenu == hPtMenu) && ptmenu->bTimeToHide) { + MENU_ReleaseMenu(ptmenu); return 0; + } } if( GetMenu(ptmenu->hWnd) == hPtMenu ) ptmenu->bTimeToHide = TRUE; + MENU_ReleaseMenu(ptmenu); } return -1; } @@ -2669,6 +2798,7 @@ static BOOL MENU_MouseMove( MTRACKER* pmt, HMENU hPtMenu, UINT wFlags ) MENU_SwitchTracking( pmt, hPtMenu, id, wFlags ); pmt->hCurrentMenu = MENU_ShowSubPopup(pmt->hOwnerWnd, hPtMenu, FALSE, wFlags); } + MENU_ReleaseMenu(ptmenu); return TRUE; } @@ -2736,10 +2866,14 @@ static LRESULT MENU_DoNextMenu( MTRACKER* pmt, UINT vk, UINT wFlags ) { /* switch to the menu bar */ - if(style & WS_CHILD || !(hNewMenu = GetMenu(hNewWnd))) return FALSE; + if(style & WS_CHILD || !(hNewMenu = GetMenu(hNewWnd))) { + MENU_ReleaseMenu(menu); + return FALSE; + } if( vk == VK_LEFT ) { + MENU_ReleaseMenu(menu); menu = MENU_GetMenu( hNewMenu ); id = menu->nItems - 1; @@ -2755,7 +2889,10 @@ static LRESULT MENU_DoNextMenu( MTRACKER* pmt, UINT vk, UINT wFlags ) /* switch to the system menu */ hNewMenu = get_win_sys_menu( hNewWnd ); } - else return FALSE; + else { + MENU_ReleaseMenu(menu); + return FALSE; + } } else /* application returned a new menu to switch to */ { @@ -2778,11 +2915,16 @@ static LRESULT MENU_DoNextMenu( MTRACKER* pmt, UINT vk, UINT wFlags ) * perhaps try to track hNewMenu as a popup? */ TRACE(" -- got confused.\n"); + MENU_ReleaseMenu(menu); return FALSE; } } - else return FALSE; + else { + MENU_ReleaseMenu(menu); + return FALSE; + } } + MENU_ReleaseMenu(menu); if( hNewMenu != pmt->hTopMenu ) { @@ -2874,6 +3016,7 @@ static BOOL MENU_KeyEscape(MTRACKER* pmt, UINT wFlags) pmt->hCurrentMenu = hmenuprev; bEndMenu = FALSE; } + MENU_ReleaseMenu(menu); } return bEndMenu; @@ -2899,6 +3042,7 @@ static void MENU_KeyLeft( MTRACKER* pmt, UINT wFlags ) MENU_SelectItem( pmt->hOwnerWnd, pmt->hCurrentMenu, prevcol, TRUE, 0 ); + MENU_ReleaseMenu(menu); return; } @@ -2929,6 +3073,8 @@ static void MENU_KeyLeft( MTRACKER* pmt, UINT wFlags ) pmt->hTopMenu, TRUE, wFlags); } } + + MENU_ReleaseMenu(menu); } @@ -2941,13 +3087,16 @@ static void MENU_KeyRight( MTRACKER* pmt, UINT wFlags ) { HMENU hmenutmp; POPUPMENU *menu = MENU_GetMenu( pmt->hTopMenu ); + POPUPMENU *curmenu = MENU_GetMenu( pmt->hCurrentMenu ); UINT nextcol; TRACE("MENU_KeyRight called, cur %p (%s), top %p (%s).\n", pmt->hCurrentMenu, - debugstr_w((MENU_GetMenu(pmt->hCurrentMenu))->items[0].text), + debugstr_w(curmenu->items[0].text), pmt->hTopMenu, debugstr_w(menu->items[0].text) ); + MENU_ReleaseMenu(curmenu); + if ( (menu->wFlags & MF_POPUP) || (pmt->hCurrentMenu != pmt->hTopMenu)) { /* If already displaying a popup, try to display sub-popup */ @@ -2956,7 +3105,10 @@ static void MENU_KeyRight( MTRACKER* pmt, UINT wFlags ) pmt->hCurrentMenu = MENU_ShowSubPopup(pmt->hOwnerWnd, hmenutmp, TRUE, wFlags); /* if subpopup was displayed then we are done */ - if (hmenutmp != pmt->hCurrentMenu) return; + if (hmenutmp != pmt->hCurrentMenu) { + MENU_ReleaseMenu(menu); + return; + } } /* Check to see if there's another column */ @@ -2965,6 +3117,7 @@ static void MENU_KeyRight( MTRACKER* pmt, UINT wFlags ) TRACE("Going to %d.\n", nextcol ); MENU_SelectItem( pmt->hOwnerWnd, pmt->hCurrentMenu, nextcol, TRUE, 0 ); + MENU_ReleaseMenu(menu); return; } @@ -2985,6 +3138,7 @@ static void MENU_KeyRight( MTRACKER* pmt, UINT wFlags ) pmt->hCurrentMenu = MENU_ShowSubPopup(pmt->hOwnerWnd, pmt->hTopMenu, TRUE, wFlags); } + MENU_ReleaseMenu(menu); } static void CALLBACK release_capture( BOOL __normal ) @@ -3040,6 +3194,7 @@ static BOOL MENU_TrackMenu( HMENU hmenu, UINT wFlags, INT x, INT y, __TRY while (!fEndMenu) { + MENU_ReleaseMenu(menu); menu = MENU_GetMenu( mt.hCurrentMenu ); if (!menu) /* sometimes happens if I do a window manager close */ break; @@ -3171,7 +3326,7 @@ static BOOL MENU_TrackMenu( HMENU hmenu, UINT wFlags, INT x, INT y, case VK_UP: case VK_DOWN: /* If on menu bar, pull-down the menu */ - + MENU_ReleaseMenu(menu); menu = MENU_GetMenu( mt.hCurrentMenu ); if (!(menu->wFlags & MF_POPUP)) mt.hCurrentMenu = MENU_ShowSubPopup(mt.hOwnerWnd, mt.hTopMenu, TRUE, wFlags); @@ -3262,6 +3417,8 @@ static BOOL MENU_TrackMenu( HMENU hmenu, UINT wFlags, INT x, INT y, } __FINALLY( release_capture ) + MENU_ReleaseMenu(menu); + /* If dropdown is still painted and the close box is clicked on then the menu will be destroyed as part of the DispatchMessage above. This will then invalidate the menu handle in mt.hTopMenu. We should @@ -3289,6 +3446,8 @@ static BOOL MENU_TrackMenu( HMENU hmenu, UINT wFlags, INT x, INT y, /* Reset the variable for hiding menu */ if( menu ) menu->bTimeToHide = FALSE; + + MENU_ReleaseMenu(menu); } /* The return value is only used by TrackPopupMenu */ @@ -3337,6 +3496,8 @@ static BOOL MENU_InitTracking(HWND hWnd, HMENU hMenu, BOOL bPopup, UINT wFlags) */ } + MENU_ReleaseMenu(menu); + return TRUE; } @@ -3467,6 +3628,7 @@ BOOL WINAPI TrackPopupMenuEx( HMENU hMenu, UINT wFlags, INT x, INT y, if (IsWindow(menu->hWnd)) { + MENU_ReleaseMenu(menu); SetLastError( ERROR_POPUP_ALREADY_ACTIVE ); return FALSE; } @@ -3494,6 +3656,7 @@ BOOL WINAPI TrackPopupMenuEx( HMENU hMenu, UINT wFlags, INT x, INT y, MAKELPARAM(0, IS_SYSTEM_MENU(menu)) ); } } + MENU_ReleaseMenu(menu); return ret; } @@ -3591,6 +3754,7 @@ UINT MENU_GetMenuBarHeight( HWND hwnd, UINT menubarWidth, HDC hdc; RECT rectBar; LPPOPUPMENU lppop; + UINT ret; TRACE("HWND %p, width %d, at (%d, %d).\n", hwnd, menubarWidth, orgX, orgY ); @@ -3601,7 +3765,9 @@ UINT MENU_GetMenuBarHeight( HWND hwnd, UINT menubarWidth, SetRect(&rectBar, orgX, orgY, orgX+menubarWidth, orgY+GetSystemMetrics(SM_CYMENU)); MENU_MenuBarCalcSize( hdc, &rectBar, lppop, hwnd ); ReleaseDC( hwnd, hdc ); - return lppop->Height; + ret = lppop->Height; + MENU_ReleaseMenu(lppop); + return ret; } @@ -3676,8 +3842,10 @@ BOOL WINAPI EnableMenuItem( HMENU hMenu, UINT wItemID, UINT wFlags ) if (!(menu = MENU_GetMenu(hMenu))) return (UINT)-1; - if (!(item = MENU_FindItem( &hMenu, &wItemID, wFlags ))) + if (!(item = MENU_FindItem( &hMenu, &wItemID, wFlags ))) { + MENU_ReleaseMenu(menu); return (UINT)-1; + } oldflags = item->fState & (MF_GRAYED | MF_DISABLED); item->fState ^= (oldflags ^ wFlags) & (MF_GRAYED | MF_DISABLED); @@ -3691,16 +3859,20 @@ BOOL WINAPI EnableMenuItem( HMENU hMenu, UINT wItemID, UINT wFlags ) POPUPMENU* parentMenu; /* Get the parent menu to access*/ - if (!(parentMenu = MENU_GetMenu(menu->hSysMenuOwner))) + if (!(parentMenu = MENU_GetMenu(menu->hSysMenuOwner))) { + MENU_ReleaseMenu(menu); return (UINT)-1; + } /* Refresh the frame to reflect the change */ WIN_GetRectangles( parentMenu->hWnd, COORDS_CLIENT, &rc, NULL ); rc.bottom = 0; RedrawWindow(parentMenu->hWnd, &rc, 0, RDW_FRAME | RDW_INVALIDATE | RDW_NOCHILDREN); + MENU_ReleaseMenu(parentMenu); } } + MENU_ReleaseMenu(menu); return oldflags; } @@ -3767,9 +3939,13 @@ BOOL WINAPI HiliteMenuItem( HWND hWnd, HMENU hMenu, UINT wItemID, TRACE("(%p, %p, %04x, %04x);\n", hWnd, hMenu, wItemID, wHilite); if (!MENU_FindItem( &hMenu, &wItemID, wHilite )) return FALSE; if (!(menu = MENU_GetMenu(hMenu))) return FALSE; - if (menu->FocusedItem == wItemID) return TRUE; + if (menu->FocusedItem == wItemID) { + MENU_ReleaseMenu(menu); + return TRUE; + } MENU_HideSubPopups( hWnd, hMenu, FALSE, 0 ); MENU_SelectItem( hWnd, hMenu, wItemID, TRUE, 0 ); + MENU_ReleaseMenu(menu); return TRUE; } @@ -3803,10 +3979,13 @@ UINT WINAPI GetMenuState( HMENU hMenu, UINT wItemID, UINT wFlags ) */ INT WINAPI GetMenuItemCount( HMENU hMenu ) { + INT ret; LPPOPUPMENU menu = MENU_GetMenu(hMenu); if (!menu) return -1; TRACE("(%p) returning %d\n", hMenu, menu->nItems ); - return menu->nItems; + ret = menu->nItems; + MENU_ReleaseMenu(menu); + return ret; } @@ -3856,7 +4035,7 @@ static void MENU_mnu2mnuii( UINT flags, UINT_PTR id, LPCWSTR str, pmii->fMask |= MIIM_DATA; pmii->dwItemData = (ULONG_PTR) str; } - if( flags & MF_POPUP && MENU_GetMenu((HMENU)id)) { + if( flags & MF_POPUP && IsMenu((HMENU)id)) { pmii->fMask |= MIIM_SUBMENU; pmii->hSubMenu = (HMENU)id; } @@ -3952,8 +4131,10 @@ BOOL WINAPI RemoveMenu( HMENU hMenu, UINT nPos, UINT wFlags ) if (!(menu = MENU_GetMenu(hMenu))) return FALSE; /* Remove item */ - if ((item->fType & MF_POPUP) && item->submenu) + if ((item->fType & MF_POPUP) && item->submenu) { MENU_ReleaseMenu(item->submenu); + item->submenu = NULL; + } MENU_FreeItemData( item ); @@ -3973,6 +4154,7 @@ BOOL WINAPI RemoveMenu( HMENU hMenu, UINT nPos, UINT wFlags ) menu->items = HeapReAlloc( GetProcessHeap(), 0, menu->items, menu->nItems * sizeof(MENUITEM) ); } + MENU_ReleaseMenu(menu); return TRUE; } @@ -3999,6 +4181,7 @@ BOOL WINAPI ModifyMenuW( HMENU hMenu, UINT pos, UINT flags, { MENUITEM *item; MENUITEMINFOW mii; + POPUPMENU *menu; if (IS_STRING_ITEM(flags)) TRACE("%p %d %04x %04lx %s\n", hMenu, pos, flags, id, debugstr_w(str) ); @@ -4006,7 +4189,9 @@ BOOL WINAPI ModifyMenuW( HMENU hMenu, UINT pos, UINT flags, TRACE("%p %d %04x %04lx %p\n", hMenu, pos, flags, id, str ); if (!(item = MENU_FindItem( &hMenu, &pos, flags ))) return FALSE; - MENU_GetMenu(hMenu)->Height = 0; /* force size recalculate */ + menu = MENU_GetMenu(hMenu); + menu->Height = 0; /* force size recalculate */ + MENU_ReleaseMenu(menu); MENU_mnu2mnuii( flags, id, str, &mii); return SetMenuItemInfo_common( item, &mii, TRUE); } @@ -4048,6 +4233,7 @@ HMENU WINAPI CreatePopupMenu(void) menu = MENU_GetMenu( hmenu ); menu->wFlags |= MF_POPUP; menu->bTimeToHide = FALSE; + MENU_ReleaseMenu(menu); return hmenu; } @@ -4200,8 +4386,10 @@ HMENU WINAPI GetSystemMenu( HWND hWnd, BOOL bRevert ) /* Store the dummy sysmenu handle to facilitate the refresh */ /* of the close button if the SC_CLOSE item change */ menu = MENU_GetMenu(retvalue); - if ( menu ) + if ( menu ) { menu->hSysMenuOwner = wndPtr->hSysMenu; + MENU_ReleaseMenu(menu); + } } WIN_ReleasePtr( wndPtr ); } @@ -4285,8 +4473,10 @@ BOOL WINAPI GetMenuBarInfo( HWND hwnd, LONG idObject, LONG idItem, PMENUBARINFO menu = MENU_GetMenu(hmenu); if (!menu) return FALSE; - if (idItem < 0 || idItem > menu->nItems) + if (idItem < 0 || idItem > menu->nItems) { + MENU_ReleaseMenu(menu); return FALSE; + } if (!menu->Height) { @@ -4311,9 +4501,8 @@ BOOL WINAPI GetMenuBarInfo( HWND hwnd, LONG idObject, LONG idItem, PMENUBARINFO pmbi->fFocused = menu->FocusedItem == idItem - 1; if (pmbi->fFocused && (menu->items[idItem - 1].fType & MF_POPUP)) { - menu = menu->items[idItem - 1].submenu; - if (menu) - pmbi->hwndMenu = menu->hWnd; + if (menu->items[idItem - 1].submenu) + pmbi->hwndMenu = menu->items[idItem - 1].submenu->hWnd; } } else @@ -4321,6 +4510,8 @@ BOOL WINAPI GetMenuBarInfo( HWND hwnd, LONG idObject, LONG idItem, PMENUBARINFO pmbi->fFocused = pmbi->fBarFocused; } + MENU_ReleaseMenu(menu); + return TRUE; } @@ -4354,6 +4545,7 @@ BOOL MENU_SetMenu( HWND hWnd, HMENU hMenu ) lpmenu->hWnd = hWnd; lpmenu->Height = 0; /* Make sure we recalculate the size */ + MENU_ReleaseMenu(lpmenu); } SetWindowLongPtrW( hWnd, GWLP_ID, (LONG_PTR)hMenu ); return TRUE; @@ -4401,6 +4593,7 @@ BOOL WINAPI DrawMenuBar( HWND hWnd ) if ((hMenu = GetMenu( hWnd )) && (lppop = MENU_GetMenu( hMenu ))) { lppop->Height = 0; /* Make sure we call MENU_MenuBarCalcSize */ lppop->hwndOwner = hWnd; + MENU_ReleaseMenu(lppop); } return SetWindowPos( hWnd, 0, 0, 0, 0, 0, SWP_NOSIZE | SWP_NOMOVE | @@ -4434,6 +4627,7 @@ DWORD WINAPI DrawMenuBarTemp(HWND hwnd, HDC hDC, LPRECT lprect, HMENU hMenu, HFO lppop = MENU_GetMenu( hMenu ); if (lppop == NULL || lprect == NULL) { + MENU_ReleaseMenu(lppop); retvalue = GetSystemMetrics(SM_CYMENU); goto END; } @@ -4456,6 +4650,7 @@ DWORD WINAPI DrawMenuBarTemp(HWND hwnd, HDC hDC, LPRECT lprect, HMENU hMenu, HFO if (lppop->nItems == 0) { retvalue = GetSystemMetrics(SM_CYMENU); + MENU_ReleaseMenu(lppop); goto END; } @@ -4465,6 +4660,7 @@ DWORD WINAPI DrawMenuBarTemp(HWND hwnd, HDC hDC, LPRECT lprect, HMENU hMenu, HFO hDC, &lppop->items[i], lppop->Height, TRUE, ODA_DRAWENTIRE ); } retvalue = lppop->Height; + MENU_ReleaseMenu(lppop); END: if (hfontOld) SelectObject (hDC, hfontOld); @@ -4577,6 +4773,8 @@ BOOL WINAPI IsMenu(HMENU hmenu) SetLastError(ERROR_INVALID_MENU_HANDLE); return FALSE; } + + MENU_ReleaseMenu(menu); return TRUE; } @@ -4819,12 +5017,12 @@ static BOOL SetMenuItemInfo_common(MENUITEM * menu, if (menu->submenu) { if( MENU_depth( menu->submenu, 0) > MAXMENUDEPTH) { ERR( "Loop detected in menu hierarchy or maximum menu depth exceeded!\n"); + MENU_ReleaseMenu(menu->submenu); menu->submenu = NULL; return FALSE; } menu->submenu->wFlags |= MF_POPUP; menu->fType |= MF_POPUP; - InterlockedIncrement(&menu->submenu->refcount); } else { SetLastError( ERROR_INVALID_PARAMETER); return FALSE; @@ -4953,6 +5151,7 @@ BOOL WINAPI SetMenuDefaultItem(HMENU hmenu, UINT uItem, UINT bypos) /* no default item */ if ( -1 == uItem) { + MENU_ReleaseMenu(menu); return TRUE; } @@ -4961,6 +5160,7 @@ BOOL WINAPI SetMenuDefaultItem(HMENU hmenu, UINT uItem, UINT bypos) { if ( uItem >= menu->nItems ) return FALSE; item[uItem].fState |= MFS_DEFAULT; + MENU_ReleaseMenu(menu); return TRUE; } else @@ -4970,11 +5170,12 @@ BOOL WINAPI SetMenuDefaultItem(HMENU hmenu, UINT uItem, UINT bypos) if (item->wID == uItem) { item->fState |= MFS_DEFAULT; + MENU_ReleaseMenu(menu); return TRUE; } } - } + MENU_ReleaseMenu(menu); return FALSE; } @@ -4995,26 +5196,39 @@ UINT WINAPI GetMenuDefaultItem(HMENU hmenu, UINT bypos, UINT flags) item = menu->items; /* empty menu */ - if (! item) return -1; + if (! item) { + MENU_ReleaseMenu(menu); + return -1; + } while ( !( item->fState & MFS_DEFAULT ) ) { i++; item++; - if (i >= menu->nItems ) return -1; + if (i >= menu->nItems ) { + MENU_ReleaseMenu(menu); + return -1; + } } /* default: don't return disabled items */ - if ( (!(GMDI_USEDISABLED & flags)) && (item->fState & MFS_DISABLED )) return -1; + if ( (!(GMDI_USEDISABLED & flags)) && (item->fState & MFS_DISABLED )) { + MENU_ReleaseMenu(menu); + return -1; + } /* search rekursiv when needed */ if ( (item->fType & MF_POPUP) && (flags & GMDI_GOINTOPOPUPS) ) { UINT ret; ret = GetMenuDefaultItem( MENU_GetHandle(item->submenu), bypos, flags ); - if ( -1 != ret ) return ret; + if ( ret != -1 ) { + MENU_ReleaseMenu(menu); + return ret; + } /* when item not found in submenu, return the popup item */ } + MENU_ReleaseMenu(menu); return ( bypos ) ? i : item->wID; } @@ -5133,9 +5347,12 @@ BOOL WINAPI GetMenuItemRect (HWND hwnd, HMENU hMenu, UINT uItem, if (itemMenu == NULL) return FALSE; - if(itemMenu->hWnd == 0) + if(itemMenu->hWnd == 0) { + MENU_ReleaseMenu(itemMenu); return FALSE; + } referenceHwnd = itemMenu->hWnd; + MENU_ReleaseMenu(itemMenu); } if ((rect == NULL) || (item == NULL)) @@ -5195,8 +5412,10 @@ BOOL WINAPI SetMenuInfo (HMENU hMenu, LPCMENUINFO lpmi) if (lpmi->dwStyle & MNS_DRAGDROP) FIXME("MNS_DRAGDROP unimplemented\n"); if (lpmi->dwStyle & MNS_MODELESS) FIXME("MNS_MODELESS unimplemented\n"); } + MENU_ReleaseMenu(menu); return TRUE; } + MENU_ReleaseMenu(menu); SetLastError( ERROR_INVALID_PARAMETER); return FALSE; } @@ -5231,6 +5450,7 @@ BOOL WINAPI GetMenuInfo (HMENU hMenu, LPMENUINFO lpmi) if (lpmi->fMask & MIM_STYLE) lpmi->dwStyle = menu->dwStyle; + MENU_ReleaseMenu(menu); return TRUE; } SetLastError( ERROR_INVALID_PARAMETER); @@ -5250,6 +5470,7 @@ BOOL WINAPI SetMenuContextHelpId( HMENU hMenu, DWORD dwContextHelpID) if ((menu = MENU_GetMenu(hMenu))) { menu->dwContextHelpID = dwContextHelpID; + MENU_ReleaseMenu(menu); return TRUE; } return FALSE; @@ -5267,7 +5488,9 @@ DWORD WINAPI GetMenuContextHelpId( HMENU hMenu ) if ((menu = MENU_GetMenu(hMenu))) { - return menu->dwContextHelpID; + DWORD ret = menu->dwContextHelpID; + MENU_ReleaseMenu(menu); + return ret; } return 0; } @@ -5282,7 +5505,11 @@ INT WINAPI MenuItemFromPoint(HWND hWnd, HMENU hMenu, POINT ptScreen) /*FIXME: Do we have to handle hWnd here? */ if (!menu) return -1; - if (!MENU_FindItemByCoords(menu, ptScreen, &pos)) return -1; + if (!MENU_FindItemByCoords(menu, ptScreen, &pos)) { + MENU_ReleaseMenu(menu); + return -1; + } + MENU_ReleaseMenu(menu); return pos; } -- 2.1.0