From 907832bd2127c257a64a4c9b04a7f500b28af20a Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 24 Mar 2014 13:06:11 +0000 Subject: [PATCH] Fix reentrancy issues in wxMSW wxTextCtrl max length handling code. The changes of r75940 didn't work correctly if the handler of wxEVT_TEXT in some text control modified a (potentially) different text control, as the same global variable was reused with disastrous results. Avoid this by keeping a stack of insertion lengths instead. Using a per-control field would work too, but would be a bit wasteful as the size of the stack will rarely be more than 1 (and never much more) and this change can also be applied to 3.0 branch without breaking ABI. Additionally, fix another problem in r75940 which used 0 as a special marker for the insertion length, which result in redoing each insertion of empty string (which is another word for Remove()) twice unnecessarily, by using -1 instead. Closes #15980. git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@76193 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- src/msw/textctrl.cpp | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/msw/textctrl.cpp b/src/msw/textctrl.cpp index af37e0d7b6..b7f25530c6 100644 --- a/src/msw/textctrl.cpp +++ b/src/msw/textctrl.cpp @@ -40,6 +40,7 @@ #include "wx/wxcrtvararg.h" #endif +#include "wx/stack.h" #include "wx/sysopt.h" #if wxUSE_CLIPBOARD @@ -176,14 +177,15 @@ private: namespace { -// The length of the text being currently inserted into the control. +// This stack stores the length of the text being currently inserted into the +// current control. // -// This is used to pass information from DoWriteText() to AdjustSpaceLimit() -// and is global as text can only be inserted into one text control at a time -// by a single thread and this operation can only be done from the main thread -// (and we don't want to waste space in every wxTextCtrl object for this field -// unnecessarily). -int gs_lenOfInsertedText = 0; +// It is used to pass information from DoWriteText() to AdjustSpaceLimit() +// and is global as text can only be inserted into a few text controls at a +// time (but possibly more than into one, if wxEVT_TEXT event handler does +// something that results in another text control update), and we don't want to +// waste space in every wxTextCtrl object for this field unnecessarily. +wxStack gs_lenOfInsertedText; } // anonymous namespace @@ -1154,29 +1156,27 @@ void wxTextCtrl::DoWriteText(const wxString& value, int flags) // Remember the length of the text we're inserting so that // AdjustSpaceLimit() could adjust the limit to be big enough for it: // and also signal us whether it did it by resetting it to 0. - gs_lenOfInsertedText = valueDos.length(); + gs_lenOfInsertedText.push(valueDos.length()); ::SendMessage(GetHwnd(), selectionOnly ? EM_REPLACESEL : WM_SETTEXT, // EM_REPLACESEL takes 1 to indicate the operation should be redoable selectionOnly ? 1 : 0, wxMSW_CONV_LPARAM(valueDos)); - if ( !gs_lenOfInsertedText ) + const int lenActuallyInserted = gs_lenOfInsertedText.top(); + gs_lenOfInsertedText.pop(); + + if ( lenActuallyInserted == -1 ) { // Text size limit has been hit and added text has been truncated. // But the max length has been increased by the EN_MAXTEXT message - // handler, which also reset gs_lenOfInsertedText to 0), so we - // should be able to set it successfully now if we try again. + // handler, which also reset the top of the lengths stack to -1), + // so we should be able to set it successfully now if we try again. if ( selectionOnly ) Undo(); ::SendMessage(GetHwnd(), selectionOnly ? EM_REPLACESEL : WM_SETTEXT, selectionOnly ? 1 : 0, wxMSW_CONV_LPARAM(valueDos)); } - else - { - // EN_MAXTEXT handler wasn't called, so reset the flag ourselves. - gs_lenOfInsertedText = 0; - } if ( !ucf.GotUpdate() && (flags & SetValue_SendEvent) ) { @@ -2159,10 +2159,10 @@ bool wxTextCtrl::AdjustSpaceLimit() // We need to increase the size of the buffer and to avoid increasing // it too many times make sure that we make it at least big enough to // fit all the text we are currently inserting into the control. - unsigned long increaseBy = gs_lenOfInsertedText; + unsigned long increaseBy = gs_lenOfInsertedText.top(); - // Don't let it affect any future unrelated calls to this function. - gs_lenOfInsertedText = 0; + // Indicate to the caller that we increased the limit. + gs_lenOfInsertedText.top() = -1; // But also increase it by at least 32KB chunks -- again, to avoid // doing it too often -- and round it up to 32KB in any case.