diff --git a/python/ycm/client/messages_request.py b/python/ycm/client/messages_request.py index d382d14456..7da8bd6a89 100644 --- a/python/ycm/client/messages_request.py +++ b/python/ycm/client/messages_request.py @@ -18,6 +18,7 @@ from ycm.client.base_request import BaseRequest, BuildRequestData from ycm.vimsupport import PostVimMessage +import json import logging _logger = logging.getLogger( __name__ ) @@ -56,8 +57,32 @@ def Poll( self, diagnostics_handler ): # Nothing yet... return True - response = self.HandleFuture( self._response_future, - display_message = False ) + # Avoid HandleFuture() to prevent blocking in timer callbacks. + # HandleFuture() does: + # 1. Complex exception handling (UnknownExtraConf, DisplayServerException) + # 2. User dialogs that can block waiting for input + # 3. Vim UI updates during callback execution + # By extracting the response directly with minimal error handling, we avoid + # blocking vim's main thread. Note that response.read() is still technically + # blocking, but: + # - The future is already done(), data received from localhost ycmd server + # - Network I/O is complete, read() just copies from buffer to memory + # - No user interaction or complex processing + # The real performance issue was HandleFuture's heavy exception handling, + # not the I/O itself. + try: + response = self._response_future.result( timeout = 0 ) + response_text = response.read() + response.close() + if response_text: + response = json.loads( response_text ) + else: + response = None + except Exception: + _logger.exception( 'Error while handling server response in Poll' ) + # Server returned an exception. + return False + if response is None: # Server returned an exception. return False diff --git a/python/ycm/tests/client/messages_request_test.py b/python/ycm/tests/client/messages_request_test.py index 1e1a10e900..dd614318ee 100644 --- a/python/ycm/tests/client/messages_request_test.py +++ b/python/ycm/tests/client/messages_request_test.py @@ -15,15 +15,16 @@ # You should have received a copy of the GNU General Public License # along with YouCompleteMe. If not, see . +import json from ycm.tests.test_utils import MockVimModule MockVimModule() from hamcrest import assert_that, equal_to from unittest import TestCase -from unittest.mock import patch, call +from unittest.mock import patch, call, MagicMock -from ycm.client.messages_request import _HandlePollResponse -from ycm.tests.test_utils import ExtendedMock +from ycm.client.messages_request import _HandlePollResponse, MessagesPoll +from ycm.tests.test_utils import ExtendedMock, MockVimBuffers, VimBuffer class MessagesRequestTest( TestCase ): @@ -138,3 +139,112 @@ def test_HandlePollResponse_MultipleMessagesAndDiagnostics( warning=False, truncate=True ), ] ) + + + def test_Poll_FirstCall_StartsRequest( self ): + test_buffer = VimBuffer( 'test_buffer', number = 1, contents = [ '' ] ) + + with MockVimBuffers( [ test_buffer ], [ test_buffer ] ): + poller = MessagesPoll( test_buffer ) + + # Mock the async request method to avoid actual HTTP call + mock_future = MagicMock() + with patch.object( poller, 'PostDataToHandlerAsync', + return_value = mock_future ) as mock_post: + # First poll should start request + result = poller.Poll( None ) + + assert_that( result, equal_to( True ) ) + mock_post.assert_called_once() + + + def test_Poll_FutureNotDone_ReturnsTrue( self ): + test_buffer = VimBuffer( 'test_buffer', number = 1, contents = [ '' ] ) + + with MockVimBuffers( [ test_buffer ], [ test_buffer ] ): + poller = MessagesPoll( test_buffer ) + + # Mock future that is not done + mock_future = MagicMock() + mock_future.done.return_value = False + poller._response_future = mock_future + + # Should return True without extracting result + result = poller.Poll( None ) + + assert_that( result, equal_to( True ) ) + mock_future.result.assert_not_called() + + + def test_Poll_FutureReady_ExtractsResponseNonBlocking( self ): + test_buffer = VimBuffer( 'test_buffer', number = 1, contents = [ '' ] ) + + with MockVimBuffers( [ test_buffer ], [ test_buffer ] ): + poller = MessagesPoll( test_buffer ) + + # Mock completed future with response + mock_response = MagicMock() + mock_response.read.return_value = json.dumps( + [ { 'message': 'test' } ] ).encode() + mock_response.close = MagicMock() + + mock_future = MagicMock() + mock_future.done.return_value = True + mock_future.result.return_value = mock_response + poller._response_future = mock_future + + # Mock diagnostics handler + mock_handler = MagicMock() + + # Should extract result with timeout=0 (non-blocking) + with patch( 'ycm.client.messages_request.PostVimMessage' ): + result = poller.Poll( mock_handler ) + + # Verify non-blocking extraction + mock_future.result.assert_called_once_with( timeout = 0 ) + mock_response.read.assert_called_once() + mock_response.close.assert_called_once() + assert_that( result, equal_to( True ) ) + + + def test_Poll_FutureException_ReturnsFalse( self ): + test_buffer = VimBuffer( 'test_buffer', number = 1, contents = [ '' ] ) + + with MockVimBuffers( [ test_buffer ], [ test_buffer ] ): + poller = MessagesPoll( test_buffer ) + + # Mock future that raises exception + mock_future = MagicMock() + mock_future.done.return_value = True + mock_future.result.side_effect = Exception( 'Connection error' ) + poller._response_future = mock_future + + # Should catch exception and return False + result = poller.Poll( None ) + + assert_that( result, equal_to( False ) ) + + + def test_Poll_DoesNotCallHandleFuture( self ): + """Verify that Poll() does NOT call HandleFuture() to avoid blocking.""" + test_buffer = VimBuffer( 'test_buffer', number = 1, contents = [ '' ] ) + + with MockVimBuffers( [ test_buffer ], [ test_buffer ] ): + poller = MessagesPoll( test_buffer ) + + # Mock completed future + mock_response = MagicMock() + mock_response.read.return_value = json.dumps( True ).encode() + mock_response.close = MagicMock() + + mock_future = MagicMock() + mock_future.done.return_value = True + mock_future.result.return_value = mock_response + poller._response_future = mock_future + + # Spy on HandleFuture to ensure it's NOT called + with patch.object( poller, 'HandleFuture' ) as mock_handle_future: + # Poll should not call HandleFuture + poller.Poll( None ) + + mock_handle_future.assert_not_called() diff --git a/python/ycm/tests/mock_utils.py b/python/ycm/tests/mock_utils.py index a200f5c0f6..ab353b584b 100644 --- a/python/ycm/tests/mock_utils.py +++ b/python/ycm/tests/mock_utils.py @@ -56,7 +56,7 @@ def done( self ): return self._done - def result( self ): + def result( self, timeout = None ): return self._result