kicad-developers team mailing list archive

[Nick Østergaard <oe.nick@xxxxxxxxx>]

Hi Maciej,

I couldn't help notice the descriptions in this email thread to
resemble https://bugs.launchpad.net/kicad/+bug/1657226

Could you check if the the issue fixed here might be present in 4.0.4 or 4.0.5?

Nick

2017-03-07 13:58 GMT+01:00 Maciej Sumiński <maciej.suminski@xxxxxxx>:
> 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
>>
>
>
>
> _______________________________________________
> 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
>