Skip to content

Fix/textinput reliability #6

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

Open
gmacmaster wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix/textinput reliability #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c21c34c
Improve TextInput reliability: thread-safe RichEdit loading and null-…
gmacmaster Apr 17, 2026
6a99432
Fix potential use-after-free in CompTextHost when TextInput is destroyed
gmacmaster Apr 17, 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
90 changes: 71 additions & 19 deletions vnext/Microsoft.ReactNative/Fabric/Composition/TextInput/WindowsTextInputComponentView.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <react/renderer/graphics/HostPlatformColor.h>
#include <react/renderer/textlayoutmanager/WindowsTextLayoutManager.h>
#include <tom.h>
#include <mutex>
#include <unicode.h>
#include <winrt/Microsoft.UI.Input.h>
#include <winrt/Windows.System.h>
Expand Down Expand Up @@ -75,37 +76,38 @@ WindowsTextInputComponentView::DrawBlock::~DrawBlock() {

// Msftedit.dll vs "Riched20.dll"?

static std::once_flag g_richEditLoadedFlag;
static HINSTANCE g_hInstRichEdit = nullptr;
static PCreateTextServices g_pfnCreateTextServices;

HRESULT HrEnsureRichEd20Loaded() noexcept {
if (g_hInstRichEdit == nullptr) {
HRESULT hr = S_OK;
std::call_once(g_richEditLoadedFlag, [&hr]() {
g_hInstRichEdit = LoadLibrary(L"Msftedit.dll");
if (!g_hInstRichEdit)
return E_FAIL;
if (!g_hInstRichEdit) {
hr = E_FAIL;
return;
}

// Create the windowless control (text services object)
g_pfnCreateTextServices = (PCreateTextServices)GetProcAddress(g_hInstRichEdit, "CreateTextServices");
if (!g_pfnCreateTextServices)
return E_FAIL;

/*
// Calling the REExtendedRegisterClass() function is required for
// registering the REComboboxW and REListBoxW window classes.
PFNREGISTER pfnRegister = (PFNREGISTER)GetProcAddress(g_hInstRichEdit, "REExtendedRegisterClass");
if (pfnRegister) {
pfnRegister();
return S_OK;
} else
return E_FAIL;
*/
}
return NOERROR;
if (!g_pfnCreateTextServices) {
hr = E_FAIL;
return;
}
});
if (!g_hInstRichEdit || !g_pfnCreateTextServices)
return E_FAIL;
return hr;
Comment on lines 83 to +101
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Apr 18, 2026

Choose a reason for hiding this comment

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

P2 call_once permanently marks failure — DLL load is never retried

std::call_once marks the flag as "done" even when the callable exits normally with hr = E_FAIL. If LoadLibrary fails on the first call, every subsequent call returns E_FAIL for the process lifetime. This is likely intentional, but a clarifying comment would help, and the return hr at the end is redundant (the globals check above already handles the failure path):

Suggested change
HRESULT HrEnsureRichEd20Loaded() noexcept {
if (g_hInstRichEdit == nullptr) {
HRESULT hr = S_OK;
std::call_once(g_richEditLoadedFlag, [&hr]() {
g_hInstRichEdit = LoadLibrary(L"Msftedit.dll");
if (!g_hInstRichEdit)
return E_FAIL;
if (!g_hInstRichEdit) {
hr = E_FAIL;
return;
}
// Create the windowless control (text services object)
g_pfnCreateTextServices = (PCreateTextServices)GetProcAddress(g_hInstRichEdit, "CreateTextServices");
if (!g_pfnCreateTextServices)
return E_FAIL;
/*
// Calling the REExtendedRegisterClass() function is required for
// registering the REComboboxW and REListBoxW window classes.
PFNREGISTER pfnRegister = (PFNREGISTER)GetProcAddress(g_hInstRichEdit, "REExtendedRegisterClass");
if (pfnRegister) {
pfnRegister();
return S_OK;
} else
return E_FAIL;
*/
}
return NOERROR;
if (!g_pfnCreateTextServices) {
hr = E_FAIL;
return;
}
});
if (!g_hInstRichEdit || !g_pfnCreateTextServices)
return E_FAIL;
return hr;
HRESULT HrEnsureRichEd20Loaded() noexcept {
// Loading is attempted exactly once per process; on failure all subsequent
// callers receive E_FAIL without retrying.
HRESULT hr = S_OK;
std::call_once(g_richEditLoadedFlag, [&hr]() {

}

struct CompTextHost : public winrt::implements<CompTextHost, ITextHost> {
CompTextHost(WindowsTextInputComponentView *outer) : m_outer(outer) {}

void Detach() {
m_outer = nullptr;
}

//@cmember Get the DC for the host
HDC TxGetDC() override {
assert(false);
Expand Down Expand Up @@ -144,13 +146,17 @@ struct CompTextHost : public winrt::implements<CompTextHost, ITextHost> {

//@cmember InvalidateRect
void TxInvalidateRect(LPCRECT prc, BOOL fMode) override {
if (!m_outer)
return;
if (m_outer->m_drawing)
return;
m_outer->DrawText();
}

//@cmember Send a WM_PAINT to the window
void TxViewChange(BOOL fUpdate) override {
if (!m_outer)
return;
// When keyboard scrolling without scrollbar, TxInvalidateRect is not called.
// Instead TxViewChange is called with fUpdate = true
// if (fUpdate && !OnInnerViewerExtentChanged())
Expand All @@ -163,12 +169,16 @@ struct CompTextHost : public winrt::implements<CompTextHost, ITextHost> {

//@cmember Create the caret
BOOL TxCreateCaret(HBITMAP hbmp, INT xWidth, INT yHeight) override {
if (!m_outer)
return false;
m_outer->m_caretVisual.Size({static_cast<float>(xWidth), static_cast<float>(yHeight)});
return true;
}

//@cmember Show the caret
BOOL TxShowCaret(BOOL fShow) override {
if (!m_outer)
return false;
// Only show the caret if we have focus
if (fShow && !m_outer->m_hasFocus) {
return false;
Expand All @@ -179,6 +189,8 @@ struct CompTextHost : public winrt::implements<CompTextHost, ITextHost> {

//@cmember Set the caret position
BOOL TxSetCaretPos(INT x, INT y) override {
if (!m_outer)
return false;
if (x < 0 && y < 0) {
// RichEdit sends (-32000,-32000) when the caret is not currently visible.
return false;
Expand Down Expand Up @@ -217,6 +229,8 @@ struct CompTextHost : public winrt::implements<CompTextHost, ITextHost> {

//@cmember Get mouse capture
void TxSetCapture(BOOL fCapture) override {
if (!m_outer)
return;
auto mousePointer = winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::Pointer>(
winrt::Microsoft::ReactNative::Composition::Input::PointerDeviceType::Mouse, 1 /* 1 is Mouse PointerId*/);

Expand All @@ -229,6 +243,8 @@ struct CompTextHost : public winrt::implements<CompTextHost, ITextHost> {

//@cmember Set the focus to the text window
void TxSetFocus() override {
if (!m_outer)
return;
winrt::Microsoft::ReactNative::ComponentView view{nullptr};
winrt::check_hresult(
m_outer->QueryInterface(winrt::guid_of<winrt::Microsoft::ReactNative::ComponentView>(), winrt::put_abi(view)));
Expand All @@ -242,11 +258,15 @@ struct CompTextHost : public winrt::implements<CompTextHost, ITextHost> {

//@cmember Establish a new cursor shape
void TxSetCursor(HCURSOR hcur, BOOL fText) override {
if (!m_outer)
return;
m_outer->m_hcursor = hcur;
}

//@cmember Converts screen coordinates of a specified point to the client coordinates
BOOL TxScreenToClient(LPPOINT lppt) override {
if (!m_outer)
return false;
winrt::Windows::Foundation::Point pt{static_cast<float>(lppt->x), static_cast<float>(lppt->y)};
pt.X -= m_outer->m_contentOffsetPx.x;
pt.Y -= m_outer->m_contentOffsetPx.y;
Expand All @@ -258,6 +278,8 @@ struct CompTextHost : public winrt::implements<CompTextHost, ITextHost> {

//@cmember Converts the client coordinates of a specified point to screen coordinates
BOOL TxClientToScreen(LPPOINT lppt) override {
if (!m_outer)
return false;
winrt::Windows::Foundation::Point pt{static_cast<float>(lppt->x), static_cast<float>(lppt->y)};

if (!m_outer->m_parent) {
Expand All @@ -284,6 +306,8 @@ struct CompTextHost : public winrt::implements<CompTextHost, ITextHost> {

//@cmember Retrieves the coordinates of a window's client area
HRESULT TxGetClientRect(LPRECT prc) override {
if (!m_outer)
return E_FAIL;
*prc = m_outer->getClientRect();

prc->top += m_outer->m_contentOffsetPx.y;
Expand All @@ -310,6 +334,8 @@ struct CompTextHost : public winrt::implements<CompTextHost, ITextHost> {

//@cmember Get the default character format for the text
HRESULT TxGetCharFormat(const CHARFORMATW **ppCF) override {
if (!m_outer)
return E_FAIL;
m_outer->UpdateCharFormat();

*ppCF = &(m_outer->m_cf);
Expand All @@ -318,6 +344,8 @@ struct CompTextHost : public winrt::implements<CompTextHost, ITextHost> {

//@cmember Get the default paragraph format for the text
HRESULT TxGetParaFormat(const PARAFORMAT **ppPF) override {
if (!m_outer)
return E_FAIL;
m_outer->UpdateParaFormat();

*ppPF = &(m_outer->m_pf);
Expand All @@ -326,6 +354,8 @@ struct CompTextHost : public winrt::implements<CompTextHost, ITextHost> {

//@cmember Get the background color for the window
COLORREF TxGetSysColor(int nIndex) override {
if (!m_outer)
return GetSysColor(nIndex);
// if (/* !m_isDisabled || */ nIndex != COLOR_WINDOW && nIndex != COLOR_WINDOWTEXT && nIndex != COLOR_GRAYTEXT) {
// This window is either not disabled or the color isn't interesting
// in the disabled case.
Expand Down Expand Up @@ -400,6 +430,10 @@ struct CompTextHost : public winrt::implements<CompTextHost, ITextHost> {

//@cmember Get the maximum length for the text
HRESULT TxGetMaxLength(DWORD *plength) override {
if (!m_outer) {
*plength = std::numeric_limits<DWORD>::max();
return S_OK;
}
auto length = m_outer->windowsTextInputProps().maxLength;
if (length > static_cast<decltype(m_outer->windowsTextInputProps().maxLength)>(std::numeric_limits<DWORD>::max())) {
length = std::numeric_limits<DWORD>::max();
Expand All @@ -410,6 +444,10 @@ struct CompTextHost : public winrt::implements<CompTextHost, ITextHost> {

//@cmember Get the bits representing requested scroll bars for the window
HRESULT TxGetScrollBars(DWORD *pdwScrollBar) override {
if (!m_outer) {
*pdwScrollBar = WS_HSCROLL | ES_AUTOHSCROLL;
return S_OK;
}
if (m_outer->m_multiline) {
*pdwScrollBar = WS_VSCROLL | WS_HSCROLL | ES_AUTOVSCROLL | ES_AUTOHSCROLL;
} else {
Expand Down Expand Up @@ -457,7 +495,8 @@ struct CompTextHost : public winrt::implements<CompTextHost, ITextHost> {

//@cmember Notify host of events
HRESULT TxNotify(DWORD iNotify, void *pv) override {
// TODO
if (!m_outer)
return S_OK;

switch (iNotify) {
case EN_UPDATE:
Expand Down Expand Up @@ -542,6 +581,16 @@ WindowsTextInputComponentView::WindowsTextInputComponentView(
reactContext,
ComponentViewFeatures::Default & ~ComponentViewFeatures::Background) {}

WindowsTextInputComponentView::~WindowsTextInputComponentView() {
// Release text services first to prevent further callbacks into CompTextHost.
m_textServices = nullptr;

// Detach the CompTextHost so any lingering references cannot use a dangling pointer.
if (m_textHost) {
winrt::get_self<CompTextHost>(m_textHost)->Detach();
}
}
Comment on lines +584 to +592
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Apr 18, 2026

Choose a reason for hiding this comment

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

P2 Callbacks during m_textServices release may reach live member functions

When m_textServices = nullptr triggers COM Release, the text-services object can still fire callbacks (e.g., TxInvalidateRect → DrawText(), or TxNotify) before the destructor returns. At that moment m_outer is still set (Detach hasn't run yet), so those callbacks enter real member functions on the partially-destroyed object. DrawText() is safe because it already guards !m_textServices (line 1781), but methods like TxSetFocus and TxSetCapture do more work. Consider calling Detach() before nulling out m_textServices so every in-flight callback immediately returns at the null-outer guard:

Suggested change
WindowsTextInputComponentView::~WindowsTextInputComponentView() {
// Release text services first to prevent further callbacks into CompTextHost.
m_textServices = nullptr;
// Detach the CompTextHost so any lingering references cannot use a dangling pointer.
if (m_textHost) {
winrt::get_self<CompTextHost>(m_textHost)->Detach();
}
}
WindowsTextInputComponentView::~WindowsTextInputComponentView() {
// Detach first so any callbacks fired during text-services teardown
// are short-circuited by the !m_outer guards.
if (m_textHost) {
winrt::get_self<CompTextHost>(m_textHost)->Detach();
}
// Now safe to release: all callbacks will see m_outer == nullptr.
m_textServices = nullptr;
}


void WindowsTextInputComponentView::HandleCommand(
const winrt::Microsoft::ReactNative::HandleCommandArgs &args) noexcept {
Super::HandleCommand(args);
Expand Down Expand Up @@ -1836,6 +1885,9 @@ WindowsTextInputComponentView::createVisual() noexcept {
winrt::com_ptr<::IUnknown> spUnk;
winrt::check_hresult(g_pfnCreateTextServices(nullptr, m_textHost.get(), spUnk.put()));
spUnk.as(m_textServices);
if (!m_textServices) {
return nullptr;
}
Comment on lines +1888 to +1890
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Apr 18, 2026

Choose a reason for hiding this comment

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

P2 Dead null-check after throwing QI

winrt::com_ptr::as() throws an hresult_error on QueryInterface failure rather than returning a null pointer, so m_textServices will never be null at this point — the if (!m_textServices) return nullptr; guard is unreachable. If you want to tolerate a failed QI gracefully instead of propagating an exception, use spUnk.try_as(m_textServices) and keep the null check; otherwise the block can be removed.

Suggested change
if (!m_textServices) {
return nullptr;
}
spUnk.as(m_textServices);


LRESULT res;
winrt::check_hresult(m_textServices->TxSendMessage(EM_SETTEXTMODE, TM_PLAINTEXT, 0, &res));
Expand Down
1 change: 1 addition & 0 deletions vnext/Microsoft.ReactNative/Fabric/Composition/TextInput/WindowsTextInputComponentView.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ struct WindowsTextInputComponentView
const winrt::Microsoft::ReactNative::Composition::Experimental::ICompositionContext &compContext,
facebook::react::Tag tag,
winrt::Microsoft::ReactNative::ReactContext const &reactContext);
~WindowsTextInputComponentView();

winrt::Microsoft::ReactNative::Composition::Experimental::IVisual createVisual() noexcept;

Expand Down