← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Fix crash when switching from dragging to routine (PNS router)

 

Hi Julius,

I apologize it has taken me so long to reply. In fact, your patch made
me investigate the problem, as I felt preventing track dragging is a
cure for the symptoms, but not for the cause.

Recently I have made a few changes to the code, and now PNS should not
crash anymore. If this is not the case, please let me know and I will
look further.

Regards,
Orson

On 03/04/2017 11:24 AM, Julius Schmidt wrote:
> What's the status on this?
> 
> FWIW, to amend my previous e-mail, I've tested the patch now and it
> works fine.
> 
> On Tue, 28 Feb 2017, Julius Schmidt wrote:
> 
>> Ok, that's closer to what an earlier patch of mine did. I was
>> wondering about the right place to put it.
>>
>> Amended version (admittedly untested).
>>
>> ---
>>  pcbnew/router/router_tool.cpp | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/pcbnew/router/router_tool.cpp
>> b/pcbnew/router/router_tool.cpp
>> index 8f79cd0..a7282c4 100644
>> --- a/pcbnew/router/router_tool.cpp
>> +++ b/pcbnew/router/router_tool.cpp
>> @@ -600,6 +600,9 @@ void ROUTER_TOOL::performRouting()
>>
>>      while( OPT_TOOL_EVENT evt = Wait() )
>>      {
>> +        // Don't crash if we missed an operation that cancelled routing.
>> +        wxCHECK2( m_router->RoutingInProgress(), break );
>> +
>>          if( evt->IsMotion() )
>>          {
>>             m_router->SetOrthoMode( evt->Modifier( MD_CTRL ) );
>> @@ -643,7 +646,8 @@ void ROUTER_TOOL::performRouting()
>>              break;
>>          }
>>          else if( TOOL_EVT_UTILS::IsCancelInteractive( *evt )
>> -                 || evt->IsUndoRedo() )
>> +                 || evt->IsUndoRedo()
>> +                 || evt->IsAction( &PCB_ACTIONS::routerInlineDrag) )
>>              break;
>>      }
>>
>> -- 
>> 2.10.2
>>
>>
>> On Tue, 28 Feb 2017, John Beard wrote:
>>
>>>  Hi Julius,
>>>
>>>  The TOOL_EVENT_UTILS functions are for generic event decision logic.
>>>  That PCB_ACTIONS::routerInlineDrag acts as to cancel an in progress
>>>  routing is specific to the ROUTER_TOOL event loop, so it should
>>>  probably be OR'ed in in ROUTER_TOOL::performRouting(), like this:
>>>
>>>         else if( TOOL_EVT_UTILS::IsCancelInteractive( *evt )
>>> | |  evt->IsAction( &PCB_ACTIONS::routerInlineDrag )
>>> | |  evt->IsUndoRedo() )
>>>
>>>  As you can see UndoRedo also has a special handling in the router tool
>>>  that is not shared with all other tools by inclusion in
>>>  IsCancelInteractive.
>>>
>>>  Cheers,
>>>
>>>  John
>>>
>>>  On Tue, Feb 28, 2017 at 9:10 AM, Julius Schmidt <aiju@xxxxxxxxxx>
>>> wrote:
>>> >  The attached patch fixes a bug where triggering InlineDrag while
>>> routing
>>> >  is in progress will crash pcbnew.  The problem is that the InlineDrag
>>> >  event does not terminate performRouting.  Once InlineDrag is finished
>>> >  it will call StopRouting which deletes the m_placer.  The Wait() in
>>> >  performRouting will then return and it will crash as soon as it tries
>>> >  to access the m_placer.
>>> > >  My fix is to add the InlineDrag action to IsCancelInteractive. I
>>> also
>>> >  added a wxCHECK2 call in case there are other ways for this to happen
>>> >  (at least it won't crash).
>>> > >  To reproduce the bug (assuming standard hotkeys):
>>> >  1. Press X to enter route mode.
>>> >  2. Click on a trace.
>>> >  3. With the cursor over a trace, press D to enter drag mode.
>>> >  4. Click somewhere to quit drag mode.
>>> >  5. (Back in routing mode) Click again to crash pcbnew.
>>> > >  ---
>>> >   pcbnew/router/router_tool.cpp     | 3 +++
>>> >   pcbnew/tools/tool_event_utils.cpp | 3 ++-
>>> >   2 files changed, 5 insertions(+), 1 deletion(-)
>>> > >  diff --git a/pcbnew/router/router_tool.cpp > 
>>> b/pcbnew/router/router_tool.cpp
>>> >  index 8f79cd0..5bfcc4c 100644
>>> >  --- a/pcbnew/router/router_tool.cpp
>>> >  +++ b/pcbnew/router/router_tool.cpp
>>> >  @@ -600,6 +600,9 @@ void ROUTER_TOOL::performRouting()
>>> > >       while( OPT_TOOL_EVENT evt = Wait() )
>>> >       {
>>> >  +        // Don't crash if IsCancelInteractive missed an operation
>>> that
>>> >  cancelled routing.
>>> >  +        wxCHECK2( m_router->RoutingInProgress(), break );
>>> >  +
>>> >           if( evt->IsMotion() )
>>> >           {
>>> >               m_router->SetOrthoMode( evt->Modifier( MD_CTRL ) );
>>> >  diff --git a/pcbnew/tools/tool_event_utils.cpp
>>> >  b/pcbnew/tools/tool_event_utils.cpp
>>> >  index 7abe83a..7d4558c 100644
>>> >  --- a/pcbnew/tools/tool_event_utils.cpp
>>> >  +++ b/pcbnew/tools/tool_event_utils.cpp
>>> >  @@ -31,7 +31,8 @@ bool TOOL_EVT_UTILS::IsCancelInteractive( const
>>> >  TOOL_EVENT& aEvt )
>>> >   {
>>> >       return aEvt.IsAction( &ACTIONS::cancelInteractive )
>>> > | |  aEvt.IsActivate()
>>> >  -            || aEvt.IsCancel();
>>> >  +            || aEvt.IsCancel()
>>> >  +            || aEvt.IsAction( &PCB_ACTIONS::routerInlineDrag );
>>> >   }
>>> > >  bool TOOL_EVT_UTILS::IsRotateToolEvt( const TOOL_EVENT& aEvt )
>>> >  --
>>> >  2.10.2
>>> > >  _______________________________________________
>>> >  Mailing list: https://launchpad.net/~kicad-developers
>>> >  Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>> >  Unsubscribe : https://launchpad.net/~kicad-developers
>>> >  More help   : https://help.launchpad.net/ListHelp
>>>
>>
>> _______________________________________________
>> Mailing list: https://launchpad.net/~kicad-developers
>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help   : https://help.launchpad.net/ListHelp
>>
>>
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp


Attachment: signature.asc
Description: OpenPGP digital signature


Follow ups

References