From 361458534cfdbf558cb1cbe70766f1cc4aeb195d Mon Sep 17 00:00:00 2001 From: Ben Jackson Date: Thu, 20 Dec 2018 15:10:03 +0000 Subject: [PATCH 1/5] Fix re-use of name result --- python3/vimspector/output.py | 2 +- python3/vimspector/variables.py | 13 +++++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/python3/vimspector/output.py b/python3/vimspector/output.py index 1637384..4bb40d3 100644 --- a/python3/vimspector/output.py +++ b/python3/vimspector/output.py @@ -90,7 +90,7 @@ class OutputView( object ): 'Evaluated: ' + expression ) result = message[ 'body' ][ 'result' ] - if message[ 'body' ].get( 'result' ) is None: + if result is None: result = 'null' utils.AppendToBuffer( console, ' Result: ' + result ) diff --git a/python3/vimspector/variables.py b/python3/vimspector/variables.py index fd4bbfd..423cdf7 100644 --- a/python3/vimspector/variables.py +++ b/python3/vimspector/variables.py @@ -294,9 +294,11 @@ class VariablesView( object ): icon = '+' if ( result.get( 'variablesReference', 0 ) > 0 and '_variables' not in result ) else '-' - line = '{0}{1} Result: {2} '.format( ' ' * indent, - icon, - result[ 'result' ] ) + result_str = result[ 'result' ] + if result_str is None: + result_str = 'null' + + line = '{0}{1} Result: {2} '.format( ' ' * indent, icon, result_str ) line = utils.AppendToBuffer( self._watch.win.buffer, line.split( '\n' ) ) self._watch.lines[ line ] = result @@ -344,9 +346,12 @@ class VariablesView( object ): # TODO: this result count be expandable, but we have no way to allow the # user to interact with the balloon to expand it. body = message[ 'body' ] + result = body[ 'result' ] + if result is None: + result = 'null' display = [ 'Type: ' + body.get( 'type', '' ), - 'Value: ' + body[ 'result' ] + 'Value: ' + result ] vim.eval( "balloon_show( {0} )".format( json.dumps( display ) ) ) From af338669f3dca726d3c1a8a1d2934d4faf61fa43 Mon Sep 17 00:00:00 2001 From: Ben Jackson Date: Thu, 20 Dec 2018 15:10:24 +0000 Subject: [PATCH 2/5] Add timeout for requests. --- autoload/vimspector/internal/channel.vim | 6 ++ autoload/vimspector/internal/job.vim | 6 +- .../vimspector/debug_adapter_connection.py | 63 ++++++++++++++++--- python3/vimspector/debug_session.py | 17 ++--- 4 files changed, 77 insertions(+), 15 deletions(-) diff --git a/autoload/vimspector/internal/channel.vim b/autoload/vimspector/internal/channel.vim index eb7cc9d..c58ff64 100644 --- a/autoload/vimspector/internal/channel.vim +++ b/autoload/vimspector/internal/channel.vim @@ -42,6 +42,12 @@ function! s:_Send( msg ) abort call ch_sendraw( s:ch, a:msg ) endfunction +function! vimspector#internal#channel#Timeout( id ) abort + py3 << EOF +_vimspector_session.OnRequestTimeout( vim.eval( 'a:id' ) ) +EOF +endfunction + function! vimspector#internal#channel#StartDebugSession( config ) abort if exists( 's:ch' ) diff --git a/autoload/vimspector/internal/job.vim b/autoload/vimspector/internal/job.vim index d252629..68aa58e 100644 --- a/autoload/vimspector/internal/job.vim +++ b/autoload/vimspector/internal/job.vim @@ -55,7 +55,7 @@ endfunction function! vimspector#internal#job#StartDebugSession( config ) abort if exists( 's:job' ) - echo "Job is already running" + echom "Job is already running" return v:none endif @@ -81,6 +81,10 @@ function! vimspector#internal#job#StartDebugSession( config ) abort endfunction function! vimspector#internal#job#StopDebugSession() abort + if ! exists( 's:job' ) + return + endfunction + if job_status( s:job ) == 'run' call job_stop( s:job, 'term' ) endif diff --git a/python3/vimspector/debug_adapter_connection.py b/python3/vimspector/debug_adapter_connection.py index 1932b5c..0cddf80 100644 --- a/python3/vimspector/debug_adapter_connection.py +++ b/python3/vimspector/debug_adapter_connection.py @@ -15,13 +15,17 @@ import logging import json - -from collections import namedtuple +import vim from vimspector import utils -PendingRequest = namedtuple( 'PendingRequest', - [ 'msg', 'handler', 'failure_handler' ] ) + +class PendingRequest( object ): + def __init__( self, msg, handler, failure_handler, expiry_id ): + self.msg = msg + self.handler = handler + self.failure_handler = failure_handler + self.expiry_id = expiry_id class DebugAdapterConnection( object ): @@ -36,18 +40,40 @@ class DebugAdapterConnection( object ): self._next_message_id = 0 self._outstanding_requests = {} - def DoRequest( self, handler, msg, failure_handler=None ): + def DoRequest( self, + handler, + msg, + failure_handler=None, + timeout = 5000 ): this_id = self._next_message_id self._next_message_id += 1 msg[ 'seq' ] = this_id msg[ 'type' ] = 'request' + # TODO/FIXME: This is so messy + expiry_id = vim.eval( + 'timer_start( {}, "vimspector#internal#channel#Timeout" )'.format( + timeout ) ) + self._outstanding_requests[ this_id ] = PendingRequest( msg, handler, - failure_handler ) + failure_handler, + expiry_id ) self._SendMessage( msg ) + def OnRequestTimeout( self, timer_id ): + request_id = None + for seq, request in self._outstanding_requests.items(): + if request.expiry_id == timer_id: + self._AbortRequest( request, 'Timeout' ) + request_id = seq + break + + # Avoid modifying _outstanding_requests while looping + if request_id is not None: + del self._outstanding_requests[ request_id ] + def DoResponse( self, request, error, response ): this_id = self._next_message_id self._next_message_id += 1 @@ -69,6 +95,21 @@ class DebugAdapterConnection( object ): def Reset( self ): self._Write = None self._handler = None + for _, request in self._outstanding_requests.items(): + self._AbortRequest( request, 'Closing down' ) + self._outstanding_requests.clear() + + def _AbortRequest( self, request, reason ): + self._logger.debug( 'Aborting request {} because {}'.format( + json.dumps( request.msg ), + reason ) ) + + _KillTimer( request ) + if request.failure_handler: + request.failure_handler( reason, {} ) + else: + utils.UserMessage( 'Request aborted: {}'.format( reason ) ) + def OnData( self, data ): data = bytes( data, 'utf-8' ) @@ -170,6 +211,8 @@ class DebugAdapterConnection( object ): self._logger.exception( 'Duplicate response: {}'.format( message ) ) return + _KillTimer( request ) + if message[ 'success' ]: if request.handler: request.handler( message ) @@ -181,7 +224,7 @@ class DebugAdapterConnection( object ): # TODO: Actually make this work reason = fmt else: - message = 'No reason' + reason = 'No reason' self._logger.error( 'Request failed: {0}'.format( reason ) ) if request.failure_handler: @@ -203,3 +246,9 @@ class DebugAdapterConnection( object ): utils.UserMessage( 'Unhandled request: {0}'.format( message[ 'command' ] ), persist = True ) + + +def _KillTimer( request ): + if request.expiry_id is not None: + vim.eval( 'timer_stop( {} )'.format( request.expiry_id ) ) + request.expiry_id = None diff --git a/python3/vimspector/debug_session.py b/python3/vimspector/debug_session.py index f3b9cd0..1019b33 100644 --- a/python3/vimspector/debug_session.py +++ b/python3/vimspector/debug_session.py @@ -189,6 +189,10 @@ class DebugSession( object ): if self._connection: self._connection.OnData( data ) + def OnRequestTimeout( self, timer_id ): + if self._connection: + self._connection.OnRequestTimeout( timer_id ) + def OnChannelClosed( self ): self._connection = None @@ -384,7 +388,7 @@ class DebugSession( object ): # scope) state = { 'done': False } - def handler( self ): + def handler( *args ): state[ 'done' ] = True self._connection.DoRequest( handler, { @@ -392,11 +396,10 @@ class DebugSession( object ): 'arguments': { 'terminateDebugee': True }, - } ) + }, failure_handler = handler ) - tries = 0 - while not state[ 'done' ] and tries < 10: - tries = tries + 1 + # This request times out after 5 seconds + while not state[ 'done' ]: vim.eval( 'vimspector#internal#{}#ForceRead()'.format( self._connection_type ) ) @@ -404,7 +407,7 @@ class DebugSession( object ): self._connection_type ) ) def _StopDebugAdapter( self, callback = None ): - def handler( message ): + def handler( *args ): vim.eval( 'vimspector#internal#{}#StopDebugSession()'.format( self._connection_type ) ) @@ -423,7 +426,7 @@ class DebugSession( object ): 'arguments': { 'terminateDebugee': True }, - } ) + }, failure_handler = handler ) def _SelectProcess( self, adapter_config, launch_config ): atttach_config = adapter_config[ 'attach' ] From ea1962e11ba81e99d743c30c1671d9cc8828f22a Mon Sep 17 00:00:00 2001 From: Ben Jackson Date: Thu, 20 Dec 2018 16:03:22 +0000 Subject: [PATCH 3/5] Use a short timeout for closedown --- autoload/vimspector/internal/job.vim | 22 ++++++++++++------- .../vimspector/debug_adapter_connection.py | 21 +++++++++--------- python3/vimspector/debug_session.py | 4 ++-- 3 files changed, 27 insertions(+), 20 deletions(-) diff --git a/autoload/vimspector/internal/job.vim b/autoload/vimspector/internal/job.vim index 68aa58e..ef8058d 100644 --- a/autoload/vimspector/internal/job.vim +++ b/autoload/vimspector/internal/job.vim @@ -39,8 +39,13 @@ function! s:_OnClose( channel ) abort endfunction function! s:_Send( msg ) abort + if ! exists( 's:job' ) + echom "Can't send message: Job was not initialised correctly" + return + endif + if job_status( s:job ) != 'run' - echom "Server isnt running" + echom "Can't send message: Job is not running" return endif @@ -55,7 +60,7 @@ endfunction function! vimspector#internal#job#StartDebugSession( config ) abort if exists( 's:job' ) - echom "Job is already running" + echom "Not starging: Job is already running" return v:none endif @@ -72,8 +77,10 @@ function! vimspector#internal#job#StartDebugSession( config ) abort \ } \ ) + echom 'Started job, status is: ' . job_status( s:job ) + if job_status( s:job ) != 'run' - echom 'Fail whale. Job is ' . job_status( s:job ) + echom 'Unable to start job, status is: ' . job_status( s:job ) return v:none endif @@ -81,9 +88,10 @@ function! vimspector#internal#job#StartDebugSession( config ) abort endfunction function! vimspector#internal#job#StopDebugSession() abort - if ! exists( 's:job' ) + if !exists( 's:job' ) + echom "Not stopping session: Job doesn't exist" return - endfunction + endif if job_status( s:job ) == 'run' call job_stop( s:job, 'term' ) @@ -93,9 +101,7 @@ function! vimspector#internal#job#StopDebugSession() abort endfunction function! vimspector#internal#job#Reset() abort - if exists( 's:job' ) - call vimspector#internal#job#StopDebugSession() - endif + call vimspector#internal#job#StopDebugSession() endfunction function! vimspector#internal#job#ForceRead() abort diff --git a/python3/vimspector/debug_adapter_connection.py b/python3/vimspector/debug_adapter_connection.py index 0cddf80..d131f7a 100644 --- a/python3/vimspector/debug_adapter_connection.py +++ b/python3/vimspector/debug_adapter_connection.py @@ -44,7 +44,7 @@ class DebugAdapterConnection( object ): handler, msg, failure_handler=None, - timeout = 5000 ): + timeout = 15000 ): this_id = self._next_message_id self._next_message_id += 1 @@ -66,13 +66,13 @@ class DebugAdapterConnection( object ): request_id = None for seq, request in self._outstanding_requests.items(): if request.expiry_id == timer_id: - self._AbortRequest( request, 'Timeout' ) request_id = seq break # Avoid modifying _outstanding_requests while looping if request_id is not None: - del self._outstanding_requests[ request_id ] + request = self._outstanding_requests.pop( request_id ) + self._AbortRequest( request, 'Timeout' ) def DoResponse( self, request, error, response ): this_id = self._next_message_id @@ -95,20 +95,21 @@ class DebugAdapterConnection( object ): def Reset( self ): self._Write = None self._handler = None - for _, request in self._outstanding_requests.items(): + + while self._outstanding_requests: + _, request = self._outstanding_requests.popitem() self._AbortRequest( request, 'Closing down' ) - self._outstanding_requests.clear() def _AbortRequest( self, request, reason ): - self._logger.debug( 'Aborting request {} because {}'.format( - json.dumps( request.msg ), - reason ) ) - + self._logger.debug( '{}: Aborting request {}'.format( reason, + request.msg ) ) _KillTimer( request ) if request.failure_handler: request.failure_handler( reason, {} ) else: - utils.UserMessage( 'Request aborted: {}'.format( reason ) ) + utils.UserMessage( 'Request for {} aborted: {}'.format( + request.msg[ 'command' ], + reason ) ) def OnData( self, data ): diff --git a/python3/vimspector/debug_session.py b/python3/vimspector/debug_session.py index 1019b33..7f11ab6 100644 --- a/python3/vimspector/debug_session.py +++ b/python3/vimspector/debug_session.py @@ -396,7 +396,7 @@ class DebugSession( object ): 'arguments': { 'terminateDebugee': True }, - }, failure_handler = handler ) + }, failure_handler = handler, timeout = 5000 ) # This request times out after 5 seconds while not state[ 'done' ]: @@ -426,7 +426,7 @@ class DebugSession( object ): 'arguments': { 'terminateDebugee': True }, - }, failure_handler = handler ) + }, failure_handler = handler, timeout = 5000 ) def _SelectProcess( self, adapter_config, launch_config ): atttach_config = adapter_config[ 'attach' ] From 0cfe1ec99bb56ea2028ff942713c419c2703a7a8 Mon Sep 17 00:00:00 2001 From: Ben Jackson Date: Thu, 20 Dec 2018 16:55:14 +0000 Subject: [PATCH 4/5] Work around cppdbg spamming mono errors to stdout --- python3/vimspector/debug_adapter_connection.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/python3/vimspector/debug_adapter_connection.py b/python3/vimspector/debug_adapter_connection.py index d131f7a..8f475b2 100644 --- a/python3/vimspector/debug_adapter_connection.py +++ b/python3/vimspector/debug_adapter_connection.py @@ -152,6 +152,11 @@ class DebugAdapterConnection( object ): if len( parts ) > 1: headers = parts[ 0 ] for header_line in headers.split( bytes( '\r\n', 'utf-8' ) ): + if bytes( '\n', 'utf-8' ) in header_line: + # Work around bugs in cppdbg where mono spams nonesense to stdout. + # This is such a dodgyhack, but it fixes the issues. + header_line = header_line.split( bytes( '\n', 'utf-8' ) )[ -1 ] + if header_line.strip(): key, value = str( header_line, 'utf-8' ).split( ':', 1 ) self._headers[ key ] = value @@ -172,6 +177,7 @@ class DebugAdapterConnection( object ): # Skip to reading headers. Because, what else can we do. self._logger.error( 'Missing Content-Length header in: {0}'.format( json.dumps( self._headers ) ) ) + self._buffer = bytes( '', 'utf-8' ) self._SetState( 'READ_HEADER' ) return From cbff48c9593b4bb78fa8aa38651fa3fa0a46a0de Mon Sep 17 00:00:00 2001 From: Ben Jackson Date: Thu, 20 Dec 2018 16:55:28 +0000 Subject: [PATCH 5/5] Work around vim bugs in appending to buffers --- python3/vimspector/utils.py | 38 +++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/python3/vimspector/utils.py b/python3/vimspector/utils.py index d2fb6e9..f57e7fb 100644 --- a/python3/vimspector/utils.py +++ b/python3/vimspector/utils.py @@ -202,25 +202,35 @@ def AskForInput( prompt ): def AppendToBuffer( buf, line_or_lines, modified=False ): - # After clearing the buffer (using buf[:] = None) there is always a single - # empty line in the buffer object and no "is empty" method. - if len( buf ) > 1 or buf[ 0 ]: - line = len( buf ) + 1 - buf.append( line_or_lines ) - elif isinstance( line_or_lines, str ): - line = 1 - buf[-1] = line_or_lines - else: - line = 1 - buf[:] = line_or_lines - - if not modified: - buf.options[ 'modified' ] = False + try: + # After clearing the buffer (using buf[:] = None) there is always a single + # empty line in the buffer object and no "is empty" method. + if len( buf ) > 1 or buf[ 0 ]: + line = len( buf ) + 1 + buf.append( line_or_lines ) + elif isinstance( line_or_lines, str ): + line = 1 + buf[-1] = line_or_lines + else: + line = 1 + buf[:] = line_or_lines + except vim.error as e: + # There seem to be a lot of Vim bugs that lead to E351, whose help says that + # this is an internal error. Ignore the error, but write a trace to the log. + if 'E315' in str( e ): + logging.getLogger( __name__ ).exception( + 'Internal error while updating buffer' ) + else: + raise e + finally: + if not modified: + buf.options[ 'modified' ] = False # Return the first Vim line number (1-based) that we just set. return line + def ClearBuffer( buf ): buf[:] = None