Fix multiple issues in IPC using TCP sockets.
This is a substantial rewrite of the old code which brings improved
error detection, MT-safety and many other fixes.
There is also a much better test suite exercising this code now.
See #24858.
wxIPC messages are small, and each one is sent as more than one short
write() call (the code word, then the payload). With Nagle's algorithm
enabled, this write-write-read pattern makes every request/reply round
trip stall on a delayed-ACK period: a Request() over loopback measured
roughly 85ms with the default socket options and well under 1ms with
TCP_NODELAY set.
Beyond the raw latency, the slow round trips are long enough to starve
other threads using the same connection. A Request() holds the reply
serialization lock for its whole round trip, so a main thread issuing
requests in a loop kept the lock busy about 95% of the time, and a
worker thread could then wait for seconds to acquire it, since mutex
wakeups are not fair. On slower machines this made the multithreaded
IPC test intermittently exceed its 30 second deadlock watchdog.
Set TCP_NODELAY on both ends of the connection: small-message
request/reply protocols are the textbook case for disabling Nagle. The
option is harmlessly refused on the Unix domain sockets used when the
server name is a path, as there is no Nagle to disable there.
This also cuts the IPC test suite runtime to roughly a third.
Request() may be called concurrently from several threads, and the
fixed-size MAX_MSG_BUFFERS ring shared by all of them could free a
buffer while the thread it was returned to was still reading it: the
number of intervening allocations needed to wrap the ring is arbitrary,
so no ring size is actually safe.
Instead, tie each buffer's lifetime to the thread consuming it:
- The buffer filled by ReadSizeAndData() is now owned by the message
object itself, so data passed to OnExecute()/OnPoke()/OnAdvise() is
freed with the message and remains valid for the duration of the
callback.
- Request() hands the reply buffer over to a thread-specific holder, so
the returned pointer stays valid until the next Request() on the same
thread, and requests made concurrently from different threads cannot
invalidate each other's data.
This removes MAX_MSG_BUFFERS and its arbitrary magic number entirely,
along with the wxTCPEventHandler buffer ring and the handler pointer
that the message classes only kept to reach it.
The thread-specific holder uses the same workaround for the MinGW
thread_local bug as UntranslatedStringHolder in translation.cpp.
IPCFixture set the gs_clientLoop global mid-constructor but only cleared
it in the destructor. If the constructor threw after that assignment --
which happens when REQUIRE(serverReady) fails because the re-exec'd
server did not come up in time -- the object never became alive, so the
destructor never ran to null the global, yet the m_clientLoop member was
still destroyed during stack unwinding. That left gs_clientLoop dangling
at a freed wxEventLoop (and leaked gs_client), and the next fixture's
opening DrainPendingIPCEvents() dereferenced the freed loop. The defect
is single-threaded, so ThreadSanitizer never flagged it; AddressSanitizer
reports it as a heap-use-after-free in DrainPendingIPCEvents().
Make construction all-or-nothing with a dismissable guard that resets the
globals (and balances wxSocketBase::Shutdown()) on any early exit, and
raise the server-readiness bound to 120s under sanitizers, where the
instrumented server is much slower to start, so the timeout no longer
fires spuriously.
Returning a unique_ptr needs an explicit conversion to the base class
pointer for gcc 4.8, in order to fix the last failing CI build. A bare
std::move() is not enough, as it triggers -Wredundant-move in the
C++20 builds with newer compilers.
WaitForAdviseWorkers() skipped Wait() when a worker thread had already
finished, but a joinable thread must be joined even then to release
its resources. The skipped joins showed up as ThreadSanitizer "thread
leak" reports in the re-executed server processes when running the
IPC tests under TSan.
The concrete type of a message read from the socket is uniquely
determined by its IPC code: ReadMessageFromSocket() is the only place
creating them and it does so via GetIPCMessageFromCode(), which maps
each code to its class. So the dynamic casts performed after checking
GetIPCCode() could never fail, and their error branches were dead code.
Replace them with wxIPCMessageCast<T>(), a static_cast that still
checks the code/type invariant with an assert in debug builds. The two
wxDynamicCasts of OnMakeConnection()/OnAcceptConnection() results
remain: those virtuals are user-overridable and may return a type not
derived from wxTCPConnection, so the check there is genuinely dynamic.
Also fix a leak in wxTCPClient::MakeConnection(): with the cast moved
into the if declaration, "connection" is necessarily null in the else
branch, so deleting it leaked the object returned by
OnMakeConnection(). Delete connectionBase instead, as the server-side
counterpart in OnSocketConnection() already does.
Change GetAddressFromName() to always return "addr" at the end.
This is done just to work around a bug in gcc 4.8 which complains about
the control reaching the end of non-void function otherwise.
Don't duplicate the complex preprocessor check multiple times, just
define wxHAS_TEST_IPC_SERVER once and check for it.
Also define the helper ShouldRunTestIPCServer() function which is more
clear than using wxGetEnv() explicitly.
No real changes.
Add a test exercising the IPC-over-sockets implementation from a single
thread and from multiple threads concurrently (Execute, Request, Poke,
Advise, combined Advise+Request, and concurrent main-thread and
worker-thread Request()s).
Each test starts its own server by re-executing the test program with
WX_IPC_TEST_SERVER set and shuts it down again in the fixture teardown, so
no server process outlives a test (or disturbs the unrelated GUI tests in
test_gui). The client runs in the main Catch2 process and queries the server
for state to verify it (Catch2 macros cannot run in the server process). The
wait loops are wall-clock bounded so they behave under a GUI event loop, and
a per-fixture watchdog aborts with a diagnostic if a test ever hangs rather
than letting CI time out.
The test runs in both the console "test" and the GUI "test_gui" programs. It
is excluded from one configuration: wxQt, whose event loop does not reliably
process a cross-thread CallAfter() (a wxQt bug fixed separately).
Do not check dark mode in wxTextCtrl::CanApplyThemeBorder(). Instead,
improve the checking for themed border drawing in the WM_NCPAINT
handler.
This fixed the border appearance after switching from dark mode to the
light one.
Closes#26794.
When using wxGCDC (applies to both GDI+ and Direct2D backends)
created from wxPaintDC, the default text color is white even
in the dark mode.
Work around this by explicitly setting the text color to the window
foreground color.
Closes#26788.
This function seems to have never existed in wxPanel and was removed
from its base wxWindow class in 74ea434841 (Add a function to perform
internal processing of wxSysColourChangedEvent, 2026-07-28).
Closes#26791.
Use system colors for generic calendar header instead of blue and gray.
A separator line is drawn below the header. This makes the control look
consistent with the Win32 and WinUI 3 controls. Custom header colors are
not overwritten by system color change event.
See #25552.
Closes#26777.
Assign values for wxSYS_COLOUR_3DLIGHT and wxSYS_COLOUR_BTNHIGHLIGHT
consistent with Linux and macOS.
Also stop using wxSYS_COLOUR_BTNHIGHLIGHT for rendering disabled text in
wxGrid and wxGenericStaticText: it didn't look before and would be even
worse after this change. Instead, just draw the text in "disabled"
colour, without using drop shadow at all.
Closes#26753.
The MFC sample unconditionally defines "_AFXDLL", which means linking
the MFC dynamically. However, this is incompatible with static CRT
linking.
Therefore, do not generate the MFC sample project when building
wxWidgets with CMake and linking the CRT statically, since the build
would have failed.
Closes#26770.
wxMenu::GtkAppend() decided between GtkImageMenuItem and plain
GtkMenuItem based on mitem->GetBitmap().IsOk() at Append() time. This
breaks the common and explicitly supported idiom
menu->Append(id, label)->SetBitmap(bmp), where the bitmap is only
attached after Append() has already built the underlying widget: for
any non-stock id, GtkAppend() picked a plain GtkMenuItem, and the
later SetupBitmaps() call (from wxWindowGTK::DoPopupMenu(), triggered
whenever the menu is actually popped up) called
gtk_image_menu_item_set_image() on it, tripping the
GTK_IS_IMAGE_MENU_ITEM assertion and silently failing to attach the
bitmap.
Always build a GtkImageMenuItem for wxITEM_NORMAL (unless a stock
GTK id already provides one). An empty GtkImageMenuItem behaves
identically to a plain GtkMenuItem, so this has no effect on items
that never get a bitmap.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CacheReadString() took the length of a string straight from the .cached
file and passed len - 1 to wxCharBuffer. A stored length of 0 underflowed
to SIZE_MAX, so wxCharTypeBuffer allocated (SIZE_MAX + 1) bytes, i.e.
none, and then wrote its terminator at str[SIZE_MAX], which wraps on
64-bit to a write one byte in front of the block. The buffer was also a
byte shorter than the read that filled it, so the terminator was
overwritten and the wxString constructor went looking for one past the
end, and a large or negative length asked for a huge allocation or a read
through a null pointer.
Size the buffer as len so the terminator survives the read, as
ReadString() in zipstrm.cpp already does, reject a length that cannot
have come from CacheWriteString() or that exceeds the file, check the
read actually delivered the bytes, and build the string from the known
length rather than by scanning for a NUL.
The contents and index counts are used to reserve memory before anything
is read, so bound them against the file as well, and move the contents
loop to std::make_unique so the new early returns cannot leak, as the
index loop already does.
Fixes#26765.
Closes#26766.
ReadTGA() sized both the image and its scratch buffer from the header's
width, height and bpp without comparing them against the input stream,
and never checked how much data the bulk reads actually returned. A
22 byte file declaring a 31232x16382 image at 24bpp therefore allocated
1.5 GB twice over, and, because a short read went undetected, LoadFile()
returned true and handed back an image of the declared size that the
file never contained.
Reject dimensions whose claimed image size cannot fit the stream before
allocating anything: uncompressed types must fit exactly, while the RLE
variants are allowed up to 128:1 expansion, since each packet costs
1 + pixelSize input bytes and yields at most 128 * pixelSize output
bytes. Also check LastRead() after the header read and after each of the
three uncompressed bulk reads, so that a truncated file is reported as a
failure instead of a success, as imagpcx.cpp has done since #26624;
DecodeRLE() already validated its own reads.
Fixes#26760.
Closes#26761.
Add missing semicolons which resulted in 2 declarations being merged
into a single one in the generated HTML.
Fix user-defined operator declaration syntax.
Fixes#26754.
Closes#26764.
Don't use the old Ubuntu version, Ubuntu PPAs are simply too flaky to be
used.
Use the latest available Ubuntu image (26.04) for all builds except
wxDFB as libdirectfb-dev package is not available in this Ubuntu version
any longer, so use 24.04 for this job.
Remove special handling of Ubuntu 18.04 which is not needed any more.
Vadim Zeitlin
John Paul MattiaandVadim Zeitlin
Steve CornettandVadim Zeitlin
PBandVadim Zeitlin
Blake-MaddenandVadim Zeitlin
mcorinoandVadim Zeitlin
Paul Cornett
Yannick DuchêneandVadim Zeitlin
Maarten Bent
Andy VandijckandMaarten Bent
MarkLee131andVadim Zeitlin
alilieandVadim Zeitlin