kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #28510
Re: [PATCH] Fix crash when switching from dragging to routine (PNS router)
-
To:
<kicad-developers@xxxxxxxxxxxxxxxxxxx>
-
From:
Maciej Sumiński <maciej.suminski@xxxxxxx>
-
Date:
Tue, 7 Mar 2017 13:58:07 +0100
-
Authentication-results:
spf=pass (sender IP is 188.184.36.50) smtp.mailfrom=cern.ch; lists.launchpad.net; dkim=none (message not signed) header.d=none;lists.launchpad.net; dmarc=bestguesspass action=none header.from=cern.ch;
-
In-reply-to:
<alpine.LNX.2.00.1703071313590.29341@phi>
-
Spamdiagnosticmetadata:
NSPM
-
Spamdiagnosticoutput:
1:99
-
User-agent:
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1
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