← Back to team overview

kicad-developers team mailing list archive

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

 

You are right, I have misinterpreted the issue. With you patch applied
everything works fine. Thank you for your contribution to KiCad.

Regards,
Orson

On 03/07/2017 01:20 PM, Julius Schmidt wrote:
> I just tested origin/master and the problem can still be reproduced.
> 
> I'm not entirely sure which changes you are referring to.
> 
> My patch also does not prevent track dragging, it just breaks the
> *routing* coroutine out of its loop, which I would think you have to do
> one way or another to avoid this bug.
> 
> On Tue, 7 Mar 2017, Maciej Sumiński wrote:
> 
>> 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
>>
>>
>>
> 
> 
> _______________________________________________
> 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