[Git][wxwidgets/wxwidgets][master] 3 commits: Fix wrong destruction of wxBitmap in wxMSW wxStaticBitmap

4 views
Skip to first unread message

Vadim Zeitlin (@_VZ_)

unread,
Jan 24, 2026, 7:59:34 PMJan 24
to wx-commi...@googlegroups.com

Vadim Zeitlin pushed to branch master at wxWidgets / wxWidgets

Commits:

  • d662d5f6
    by Vadim Zeitlin at 2026-01-24T17:02:06+01:00 
    Fix wrong destruction of wxBitmap in wxMSW wxStaticBitmap

Due to a regression introduced in 3e32a9abe1 (Don't bother resetting
wxStaticBitmap image when destroying it, 2025-06-12), which was part
of #25518, the handle of wxBitmap passed to wxStaticBitmap::SetBitmap()
was destroyed when another bitmap was passed to SetBitmap() later.

Fix this by ensuring that we don't overwrite the value of the current
handle (to be deleted) prematurely in DoUpdateImage().

Add a simple unit test for wxStaticBitmap checking that the problem is
really fixed and, under MSW, also that it doesn't leak bitmap handles.

Closes #26106.
  • 9be394fe
    by Vadim Zeitlin at 2026-01-25T01:41:54+01:00 
    Make wxXmlInitResourceModule() safer to use

This function unconditionally added a new wxXmlResourceModule object to
the modules list which could result in problems if it had been already
initialized.

Ensure that we do this only if necessary by calling the new function
added to wxModule which checks if the module is already known and adds
and initializes only this module if it is not.

See #26111.

Closes #26039.
  • 8245e107
    by Vadim Zeitlin at 2026-01-25T01:43:52+01:00 
    Improve documentation of null wxDialog parent

Mention wxDIALOG_NO_PARENT in the documentation of the parent parameter
as people who don't know about this style are not going to see this
style documentation either.

See #23560.

12 changed files:

Changes:

  • include/wx/module.h
    ... ... @@ -54,6 +54,10 @@ public:
    54 54
    55 55 
         static void UnregisterModule(wxModule *module);
    56 56
    57 
    +    // Initialize the module with the given type information if it's not
    58 
    +    // already initialized. This is for internal use only currently.
    59 
    +    static void AddModuleIfNecessary(const wxClassInfo *classInfo);
    60 
    +
    57 61 
     protected:
    58 62 
         static wxModuleList ms_modules;
    59 63

  • interface/wx/dialog.h
    ... ... @@ -166,7 +166,10 @@ public:
    166 166 
             Constructor.
    167 167
    168 168 
             @param parent
    169 
    -            Can be @NULL, a frame or another dialog box.
    169 
    +            Can be @NULL, a frame or another dialog box. Please note that when
    170 
    +            the parent is @NULL, the dialog will be owned by the application's
    171 
    +            top window, if any. Use ::wxDIALOG_NO_PARENT style to really make
    172 
    +            dialog not owned by any window.
    170 173 
             @param id
    171 174 
                 An identifier for the dialog. A value of -1 is taken to mean a
    172 175 
                 default.

  • src/common/module.cpp
    ... ... @@ -18,6 +18,8 @@
    18 18 
         #include "wx/log.h"
    19 19 
     #endif
    20 20
    21 
    +#include <memory>
    22 
    +
    21 23 
     #define TRACE_MODULE wxT("module")
    22 24
    23 25 
     wxIMPLEMENT_ABSTRACT_CLASS(wxModule, wxObject);
    ... ... @@ -148,6 +150,43 @@ bool wxModule::DoInitializeModule(wxModule *module,
    148 150 
         return true;
    149 151 
     }
    150 152
    153 
    +void wxModule::AddModuleIfNecessary(const wxClassInfo *classInfo)
    154 
    +{
    155 
    +    wxCHECK_RET( classInfo, wxS("Valid class info must be provided") );
    156 
    +    wxCHECK_RET( classInfo->IsKindOf(wxCLASSINFO(wxModule)),
    157 
    +                  wxS("Class info must be for wxModule-derived class") );
    158 
    +
    159 
    +    const wxString className(classInfo->GetClassName());
    160 
    +    for ( wxModuleList::const_iterator it = ms_modules.begin();
    161 
    +          it != ms_modules.end();
    162 
    +          ++it )
    163 
    +    {
    164 
    +        if ( (*it)->GetClassInfo()->GetClassName() == className )
    165 
    +        {
    166 
    +            // Already initialized or at least registered and will be
    167 
    +            // initialized later.
    168 
    +            return;
    169 
    +        }
    170 
    +    }
    171 
    +
    172 
    +    std::unique_ptr<wxModule>
    173 
    +        module{static_cast<wxModule*>(classInfo->CreateObject())};
    174 
    +
    175 
    +    // Do not call RegisterModule() here as it would add it to ms_modules which
    176 
    +    // would result in it being added twice as DoInitializeModule() would do it
    177 
    +    // too on success.
    178 
    +    module->m_state = State_Registered;
    179 
    +
    180 
    +    if ( !DoInitializeModule(module.get(), ms_modules) )
    181 
    +    {
    182 
    +        // Error is already given by DoInitializeModule().
    183 
    +        return;
    184 
    +    }
    185 
    +
    186 
    +    // Module is now owned by ms_modules.
    187 
    +    module.release();
    188 
    +}
    189 
    +
    151 190 
     // Initialize user-defined modules
    152 191 
     bool wxModule::InitializeModules()
    153 192 
     {

  • src/msw/statbmp.cpp
    ... ... @@ -231,6 +231,7 @@ void wxStaticBitmap::DoUpdateImage(const wxSize& sizeOld, bool wasIcon)
    231 231
    232 232 
         // For the icons we just use its HICON directly, but for bitmaps we create
    233 233 
         // our own temporary bitmap and need to delete its handle manually later.
    234 
    +    WXHANDLE currentHandle = 0;
    234 235 
         if ( !m_icon.IsOk() )
    235 236 
         {
    236 237 
             wxBitmap bitmap = m_bitmapBundle.GetBitmapFor(this);
    ... ... @@ -244,8 +245,8 @@ void wxStaticBitmap::DoUpdateImage(const wxSize& sizeOld, bool wasIcon)
    244 245 
             {
    245 246 
                 // For bitmap with alpha channel create temporary DIB with
    246 247 
                 // not-premultiplied alpha values.
    247 
    -            m_currentHandle = wxDIB(bitmap.ConvertToImage(),
    248 
    -                                    wxDIB::PixelFormat_NotPreMultiplied)
    248 
    +            currentHandle = wxDIB(bitmap.ConvertToImage(),
    249 
    +                                  wxDIB::PixelFormat_NotPreMultiplied)
    249 250 
                     .Detach();
    250 251 
                 m_ownsCurrentHandle = true;
    251 252 
             }
    ... ... @@ -266,14 +267,14 @@ void wxStaticBitmap::DoUpdateImage(const wxSize& sizeOld, bool wasIcon)
    266 267 
                 // Just use the HBITMAP as is, but also make a copy of the bitmap
    267 268 
                 // to ensure that HBITMAP remains valid for as long as we need it
    268 269 
                 m_bitmap = bitmap;
    269 
    -            m_currentHandle = bitmap.GetHandle();
    270 
    +            currentHandle = bitmap.GetHandle();
    270 271 
             }
    271 272 
         }
    272 273
    273 274 
         const bool isIcon = m_icon.IsOk();
    274 275 
         if ( isIcon )
    275 276 
         {
    276 
    -        m_currentHandle = m_icon.GetHandle();
    277 
    +        currentHandle = m_icon.GetHandle();
    277 278 
         }
    278 279
    279 280 
         if ( isIcon != wasIcon )
    ... ... @@ -287,18 +288,22 @@ void wxStaticBitmap::DoUpdateImage(const wxSize& sizeOld, bool wasIcon)
    287 288 
         // Update the handle used by the native control.
    288 289 
         const WPARAM imageType = m_icon.IsOk() ? IMAGE_ICON : IMAGE_BITMAP;
    289 290
    290 
    -    const HGDIOBJ currentHandle = (HGDIOBJ)m_currentHandle;
    291 291 
         const HGDIOBJ oldHandle = (HGDIOBJ)
    292 292 
             ::SendMessage(GetHwnd(), STM_SETIMAGE, imageType, (LPARAM)currentHandle);
    293 293
    294 294 
         // detect if this is still the handle we passed before or
    295 295 
         // if the static-control made a copy of the bitmap!
    296 
    -    if ( oldHandle != 0 && oldHandle != currentHandle )
    296 
    +    if ( oldHandle != 0 && oldHandle != m_currentHandle )
    297 297 
         {
    298 298 
             // the static control made a copy and we are responsible for deleting it
    299 299 
             ::DeleteObject(oldHandle);
    300 300 
         }
    301 301
    302 
    +    // Note that m_currentHandle must be changed after comparing its previous
    303 
    +    // value with oldHandle above and before possibly freeing it in the call to
    304 
    +    // Free() below.
    305 
    +    m_currentHandle = currentHandle;
    306 
    +
    302 307 
         // Also check if we need to keep our current handle, it may be unnecessary
    303 308 
         // if the native control doesn't actually use it.
    304 309 
         const HGDIOBJ newHandle = (HGDIOBJ)

  • src/xrc/xmlres.cpp
    ... ... @@ -3255,9 +3255,17 @@ wxIMPLEMENT_DYNAMIC_CLASS(wxXmlResourceModule, wxModule);
    3255 3255 
     // then the built-in module system won't pick this one up.  Add it manually.
    3256 3256 
     void wxXmlInitResourceModule()
    3257 3257 
     {
    3258 
    -    wxModule* module = new wxXmlResourceModule;
    3259 
    -    wxModule::RegisterModule(module);
    3260 
    -    wxModule::InitializeModules();
    3258 
    +    // Only add the module if the module system is already initialized,
    3259 
    +    // otherwise it will be added automatically when it is done as it will be
    3260 
    +    // known to the module system at that time (this happens as soon as the
    3261 
    +    // code containing this function is loaded from a shared library and if
    3262 
    +    // this function is running, it must have been already loaded!).
    3263 
    +    if ( wxModule::AreInitialized() )
    3264 
    +    {
    3265 
    +        // Still check that this module is not already registered, as could
    3266 
    +        // happen if this function is called multiple times, for example.
    3267 
    +        wxModule::AddModuleIfNecessary(wxCLASSINFO(wxXmlResourceModule));
    3268 
    +    }
    3261 3269 
     }
    3262 3270
    3263 3271 
     #endif // wxUSE_XRC

  • tests/Makefile.in
    ... ... @@ -239,6 +239,7 @@ TEST_GUI_OBJECTS = \
    239 239 
     	test_gui_slidertest.o \
    240 240 
     	test_gui_spinctrldbltest.o \
    241 241 
     	test_gui_spinctrltest.o \
    242 
    +	test_gui_statbmptest.o \
    242 243 
     	test_gui_styledtextctrltest.o \
    243 244 
     	test_gui_textctrltest.o \
    244 245 
     	test_gui_textentrytest.o \
    ... ... @@ -1138,6 +1139,9 @@ test_gui_spinctrldbltest.o: $(srcdir)/controls/spinctrldbltest.cpp $(TEST_GUI_OD
    1138 1139 
     test_gui_spinctrltest.o: $(srcdir)/controls/spinctrltest.cpp $(TEST_GUI_ODEP)
    1139 1140 
     	$(CXXC) -c -o $@ $(TEST_GUI_CXXFLAGS) $(srcdir)/controls/spinctrltest.cpp
    1140 1141
    1142 
    +test_gui_statbmptest.o: $(srcdir)/controls/statbmptest.cpp $(TEST_GUI_ODEP)
    1143 
    +	$(CXXC) -c -o $@ $(TEST_GUI_CXXFLAGS) $(srcdir)/controls/statbmptest.cpp
    1144 
    +
    1141 1145 
     test_gui_styledtextctrltest.o: $(srcdir)/controls/styledtextctrltest.cpp $(TEST_GUI_ODEP)
    1142 1146 
     	$(CXXC) -c -o $@ $(TEST_GUI_CXXFLAGS) $(srcdir)/controls/styledtextctrltest.cpp
    1143 1147

  • tests/controls/statbmptest.cpp
    1 
    +///////////////////////////////////////////////////////////////////////////////
    2 
    +// Name:        tests/controls/statbmptest.cpp
    3 
    +// Purpose:     wxStaticBitmap unit test
    4 
    +// Author:      Vadim Zeitlin
    5 
    +// Created:     2026-01-22
    6 
    +// Copyright:   (c) 2026 Vadim Zeitlin
    7 
    +///////////////////////////////////////////////////////////////////////////////
    8 
    +
    9 
    +#include "testprec.h"
    10 
    +
    11 
    +#if wxUSE_STATBMP
    12 
    +
    13 
    +#ifndef WX_PRECOMP
    14 
    +    #include "wx/app.h"
    15 
    +#endif // WX_PRECOMP
    16 
    +
    17 
    +#include "wx/statbmp.h"
    18 
    +
    19 
    +#include "wx/dcmemory.h"
    20 
    +
    21 
    +#ifdef __WINDOWS__
    22 
    +    #include "wx/msw/private/resource_usage.h"
    23 
    +#endif // __WINDOWS__
    24 
    +
    25 
    +static void TestStaticBitmap(wxWindow* window, double scale)
    26 
    +{
    27 
    +    // Create the bitmaps using the window scale factor to make sure they
    28 
    +    // are used directly by wxStaticBitmap instead, without any rescaling
    29 
    +    // (which would create temporary bitmaps and hide the manifestation of
    30 
    +    // the bug that this test was written to trigger, see #26106).
    31 
    +    wxBitmap bmp1;
    32 
    +    bmp1.CreateWithDIPSize(16, 16, scale);
    33 
    +    wxBitmap bmp2;
    34 
    +    bmp2.CreateWithDIPSize(32, 32, scale);
    35 
    +
    36 
    +    wxIcon icon;
    37 
    +    icon.CopyFromBitmap(bmp1);
    38 
    +
    39 
    +    wxStaticBitmap statbmp(window, wxID_ANY, bmp1);
    40 
    +    REQUIRE( statbmp.GetBitmap().IsOk() );
    41 
    +
    42 
    +    statbmp.SetBitmap(bmp2);
    43 
    +    REQUIRE( statbmp.GetBitmap().IsOk() );
    44 
    +
    45 
    +    // Check that the bitmap is valid by selecting it into wxMemoryDC:
    46 
    +    // unlike IsOk() check, this would fail if it had been destroyed.
    47 
    +    REQUIRE_NOTHROW( wxMemoryDC(bmp1) );
    48 
    +
    49 
    +    statbmp.SetIcon(icon);
    50 
    +    REQUIRE_NOTHROW( wxMemoryDC(bmp2) );
    51 
    +
    52 
    +    statbmp.SetBitmap(bmp2);
    53 
    +    REQUIRE_NOTHROW( wxMemoryDC(bmp1) );
    54 
    +}
    55 
    +
    56 
    +TEST_CASE("wxStaticBitmap::Set", "[wxStaticBitmap][bitmap]")
    57 
    +{
    58 
    +    auto* const window = wxTheApp->GetTopWindow();
    59 
    +
    60 
    +    TestStaticBitmap(window, window->GetDPIScaleFactor());
    61 
    +}
    62 
    +
    63 
    +#ifdef __WINDOWS__
    64 
    +
    65 
    +TEST_CASE("wxStaticBitmap::ResourceLeak", "[wxStaticBitmap]")
    66 
    +{
    67 
    +    auto* const window = wxTheApp->GetTopWindow();
    68 
    +    auto const scale = window->GetDPIScaleFactor();
    69 
    +
    70 
    +    wxGUIObjectUsage usageBefore;
    71 
    +    for ( int n = 0; n < 100; ++n )
    72 
    +    {
    73 
    +        TestStaticBitmap(window, scale);
    74 
    +
    75 
    +        // First use of GDI allocates some resources, so consider the state
    76 
    +        // after the first iteration as the baseline: if there are any leaks,
    77 
    +        // we'll see them in the subsequent iterations too and the test below
    78 
    +        // will fail.
    79 
    +        if ( n == 0 )
    80 
    +            usageBefore = wxGetCurrentlyUsedResources();
    81 
    +    }
    82 
    +
    83 
    +    auto const usageAfter = wxGetCurrentlyUsedResources();
    84 
    +    CHECK( usageAfter.numGDI == usageBefore.numGDI );
    85 
    +}
    86 
    +
    87 
    +#endif // __WINDOWS__
    88 
    +
    89 
    +#endif // wxUSE_STATBMP

  • tests/makefile.gcc
    ... ... @@ -212,6 +212,7 @@ TEST_GUI_OBJECTS = \
    212 212 
     	$(OBJS)\test_gui_slidertest.o \
    213 213 
     	$(OBJS)\test_gui_spinctrldbltest.o \
    214 214 
     	$(OBJS)\test_gui_spinctrltest.o \
    215 
    +	$(OBJS)\test_gui_statbmptest.o \
    215 216 
     	$(OBJS)\test_gui_styledtextctrltest.o \
    216 217 
     	$(OBJS)\test_gui_textctrltest.o \
    217 218 
     	$(OBJS)\test_gui_textentrytest.o \
    ... ... @@ -1085,6 +1086,9 @@ $(OBJS)\test_gui_spinctrldbltest.o: ./controls/spinctrldbltest.cpp
    1085 1086 
     $(OBJS)\test_gui_spinctrltest.o: ./controls/spinctrltest.cpp
    1086 1087 
     	$(CXX) -c -o $@ $(TEST_GUI_CXXFLAGS) $(CPPDEPS) $<
    1087 1088
    1089 
    +$(OBJS)\test_gui_statbmptest.o: ./controls/statbmptest.cpp
    1090 
    +	$(CXX) -c -o $@ $(TEST_GUI_CXXFLAGS) $(CPPDEPS) $<
    1091 
    +
    1088 1092 
     $(OBJS)\test_gui_styledtextctrltest.o: ./controls/styledtextctrltest.cpp
    1089 1093 
     	$(CXX) -c -o $@ $(TEST_GUI_CXXFLAGS) $(CPPDEPS) $<
    1090 1094

  • tests/makefile.vc
    ... ... @@ -227,6 +227,7 @@ TEST_GUI_OBJECTS = \
    227 227 
     	$(OBJS)\test_gui_slidertest.obj \
    228 228 
     	$(OBJS)\test_gui_spinctrldbltest.obj \
    229 229 
     	$(OBJS)\test_gui_spinctrltest.obj \
    230 
    +	$(OBJS)\test_gui_statbmptest.obj \
    230 231 
     	$(OBJS)\test_gui_styledtextctrltest.obj \
    231 232 
     	$(OBJS)\test_gui_textctrltest.obj \
    232 233 
     	$(OBJS)\test_gui_textentrytest.obj \
    ... ... @@ -1384,6 +1385,9 @@ $(OBJS)\test_gui_spinctrldbltest.obj: .\controls\spinctrldbltest.cpp
    1384 1385 
     $(OBJS)\test_gui_spinctrltest.obj: .\controls\spinctrltest.cpp
    1385 1386 
     	$(CXX) /c /nologo /TP /Fo$@ $(TEST_GUI_CXXFLAGS) .\controls\spinctrltest.cpp
    1386 1387
    1388 
    +$(OBJS)\test_gui_statbmptest.obj: .\controls\statbmptest.cpp
    1389 
    +	$(CXX) /c /nologo /TP /Fo$@ $(TEST_GUI_CXXFLAGS) .\controls\statbmptest.cpp
    1390 
    +
    1387 1391 
     $(OBJS)\test_gui_styledtextctrltest.obj: .\controls\styledtextctrltest.cpp
    1388 1392 
     	$(CXX) /c /nologo /TP /Fo$@ $(TEST_GUI_CXXFLAGS) .\controls\styledtextctrltest.cpp
    1389 1393

  • tests/test.bkl
    ... ... @@ -252,6 +252,7 @@
    252 252 
                 controls/slidertest.cpp
    253 253 
                 controls/spinctrldbltest.cpp
    254 254 
                 controls/spinctrltest.cpp
    255 
    +            controls/statbmptest.cpp
    255 256 
                 controls/styledtextctrltest.cpp
    256 257 
                 controls/textctrltest.cpp
    257 258 
                 controls/textentrytest.cpp

  • tests/test_gui.vcxproj
    ... ... @@ -954,6 +954,7 @@
    954 954 
         <ClCompile Include="controls\slidertest.cpp" />
    955 955 
         <ClCompile Include="controls\spinctrldbltest.cpp" />
    956 956 
         <ClCompile Include="controls\spinctrltest.cpp" />
    957 
    +    <ClCompile Include="controls\statbmptest.cpp" />
    957 958 
         <ClCompile Include="controls\styledtextctrltest.cpp" />
    958 959 
         <ClCompile Include="controls\textctrltest.cpp" />
    959 960 
         <ClCompile Include="controls\textentrytest.cpp" />

  • tests/test_gui.vcxproj.filters
    ... ... @@ -248,6 +248,9 @@
    248 248 
         <ClCompile Include="controls\spinctrltest.cpp">
    249 249 
           <Filter>Source Files</Filter>
    250 250 
         </ClCompile>
    251 
    +    <ClCompile Include="controls\statbmptest.cpp">
    252 
    +      <Filter>Source Files</Filter>
    253 
    +    </ClCompile>
    251 254 
         <ClCompile Include="controls\styledtextctrltest.cpp">
    252 255 
           <Filter>Source Files</Filter>
    253 256 
         </ClCompile>

Reply all
Reply to author
Forward
0 new messages