Author Topic: Delete an active target  (Read 4903 times)

Offline LETARTARE

  • Lives here!
  • ****
  • Posts: 531
  • L'ami de l'homme.The friend of man.
    • LETARTARE
Delete an active target
« on: January 06, 2019, 07:12:53 pm »
This test was performed under Vista (win32) and  Cb-11546 compiled with wx28 and Tdm492.
The plugin CC is disabled.

The project attached 'deleteTarget.cbp' (a simple console application) can detect a crash when calls from 'MacrosManager::RecalcVars(...)' and 'MacrosManager::ReplaceMacros(...)',  under the following conditions :
All targets (Release, one, two, three) uses :
  • Output filename = bin\$TARGET_NAME)\deleteTarget.exe
  • Objects output dir = obj\$TARGET_NAME)\
Targets 'one', 'two', 'three' have more :
  • Custom variables : wx = $(#wx28_492)
  • Search directories include = $wx\include
- the global variable "#wx28_492" is not used for compilation !, just for demonstration.

All targets compile and work correctly.
  • 1- compile and run target 'debug'
  • 2- delete active target 'debug'
  • 3- it's ok
=> trying to do the same thing with another (active)  target causes a cb crash with a report (attached) !!

A note after removing the 'Debug' target : binary files and '*.o' still exist.

The oldest version without crash seems to be 'cb-11454' nightly build.
No test under Linux.



CB-13483, plugins-sdk-2.25.0 : Collector-2.0.0, AddOnForQt-3.9.1
1-Win7 Business Pack1 64bits : wx-3.2.4, gcc-8.1.0,
2-OpenSuse::Leap-15.4-64bits : wx-3.2.4;gtk3, gcc-8.2.1,
=> !! The messages are translated by Deepl

Offline BlueHazzard

  • Developer
  • Lives here!
  • *****
  • Posts: 3353
Re: Delete an active target
« Reply #1 on: January 06, 2019, 07:32:45 pm »
And here we probably have it xD  the first bug probably caused by [r11465]

Can you test if [r11464]  works, and if the bug is introduced in [r11465]?
« Last Edit: January 06, 2019, 07:52:41 pm by BlueHazzard »

Offline LETARTARE

  • Lives here!
  • ****
  • Posts: 531
  • L'ami de l'homme.The friend of man.
    • LETARTARE
Re: Delete an active target
« Reply #2 on: January 06, 2019, 07:41:00 pm »
Yes, I will download both versions and try them out...
CB-13483, plugins-sdk-2.25.0 : Collector-2.0.0, AddOnForQt-3.9.1
1-Win7 Business Pack1 64bits : wx-3.2.4, gcc-8.1.0,
2-OpenSuse::Leap-15.4-64bits : wx-3.2.4;gtk3, gcc-8.2.1,
=> !! The messages are translated by Deepl

Offline LETARTARE

  • Lives here!
  • ****
  • Posts: 531
  • L'ami de l'homme.The friend of man.
    • LETARTARE
Re: Delete an active target
« Reply #3 on: January 06, 2019, 11:23:51 pm »
Indeed 'cb11464' does not crash while 'cb11465' crash (see report).
But what do you mean "it xD" ?
CB-13483, plugins-sdk-2.25.0 : Collector-2.0.0, AddOnForQt-3.9.1
1-Win7 Business Pack1 64bits : wx-3.2.4, gcc-8.1.0,
2-OpenSuse::Leap-15.4-64bits : wx-3.2.4;gtk3, gcc-8.2.1,
=> !! The messages are translated by Deepl

Offline BlueHazzard

  • Developer
  • Lives here!
  • *****
  • Posts: 3353
Re: Delete an active target
« Reply #4 on: January 07, 2019, 05:12:49 pm »
Quote
But what do you mean "it xD" ?
Quote
And here we probably have it xD 
This was sarcastic or cynical... It was only a matter of time until someone will find a bug about this patch.

i will look into it...

Offline BlueHazzard

  • Developer
  • Lives here!
  • *****
  • Posts: 3353
Re: Delete an active target
« Reply #5 on: January 07, 2019, 07:20:04 pm »
The problem is, that the member variable "m_LastTarget" of MacrosManager points to an invalid target after the target is deleted.

The question is how to solve this issue?
1) Reset the MacrosManager if a target is deleted?
2) Only if the "current target == m_LastTarget"?
3) Reset only "m_LastTarget" if the target is deleted?

2) should probably be the solution to go..

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: Delete an active target
« Reply #6 on: January 07, 2019, 07:31:40 pm »
What do you mean by 2? I don't understand.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline BlueHazzard

  • Developer
  • Lives here!
  • *****
  • Posts: 3353
Re: Delete an active target
« Reply #7 on: January 07, 2019, 10:07:44 pm »
What do you mean by 2? I don't understand.
Code
if(&deleteTarget==&m_LastTarget)
{
MacrosManager::Reset();
}
This would be a quick fix.

A proper fix would be to use shared_ptr<> and weak_ptr<> on every place a target pointer is used...

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: Delete an active target
« Reply #8 on: January 07, 2019, 10:36:00 pm »
A proper fix would be to use shared_ptr<> and weak_ptr<> on every place a target pointer is used...
I'm not really sure this would be a good idea. For sure it would be quite hard to implement, because you'll probably have to change every file in the repo.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline BlueHazzard

  • Developer
  • Lives here!
  • *****
  • Posts: 3353
Re: Delete an active target
« Reply #9 on: January 08, 2019, 01:34:08 am »
Code
@@ -1310,10 +1310,11 @@ bool cbProject::RemoveBuildTarget(int index)
         // finally remove the target
         delete target;
         m_Targets.RemoveAt(index);
         SetModified(true);
         NotifyPlugins(cbEVT_PROJECT_TARGETS_MODIFIED);
+        Manager::Get()->GetMacrosManager()->Reset();
         return true;
     }
     return false;
 }
 

This fixes this bug...

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: Delete an active target
« Reply #10 on: January 08, 2019, 08:16:19 am »
OK, commit it...But add a comment why this line is there...
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline LETARTARE

  • Lives here!
  • ****
  • Posts: 531
  • L'ami de l'homme.The friend of man.
    • LETARTARE
Re: Delete an active target
« Reply #11 on: January 08, 2019, 10:53:56 am »
@BlueHazzard
I just checked on cb11546 modified: it does not crash anymore
Thank you.

==> I notice that all deleted targets leave old compilations in their old places.
For the sample provided as an attachment 'deleteTarget.zip'
   
Quote
    Output filename = bin\$(TARGET_NAME)\deleteTarget.exe
    Objects output dir = obj\$(TARGET_NAME)\
if  the deleted target is "one" =>
   
Quote
    Output filename = bin\one\deleteTarget.exe
    Objects output dir = obj\one\main.o
are kept on the disc !!

==> It should be noted that when we rename a target (active) we also keep on disk old compilations...


CB-13483, plugins-sdk-2.25.0 : Collector-2.0.0, AddOnForQt-3.9.1
1-Win7 Business Pack1 64bits : wx-3.2.4, gcc-8.1.0,
2-OpenSuse::Leap-15.4-64bits : wx-3.2.4;gtk3, gcc-8.2.1,
=> !! The messages are translated by Deepl

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: Delete an active target
« Reply #12 on: January 11, 2019, 07:16:02 pm »
@bluehazzard: Are you gonna commit this fix?
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline BlueHazzard

  • Developer
  • Lives here!
  • *****
  • Posts: 3353
Re: Delete an active target
« Reply #13 on: January 11, 2019, 08:51:53 pm »
i currently try to implement a test with squirrel scripting to test some cases with variables and targets, because i feel that this is all a bit fragile, and testing this complex system would not be the worst option...
But it is not that easy to implement them...

Offline LETARTARE

  • Lives here!
  • ****
  • Posts: 531
  • L'ami de l'homme.The friend of man.
    • LETARTARE
Re: Delete an active target
« Reply #14 on: January 12, 2019, 10:14:58 am »
I can test your scripts under the versions indicated in my signature.
CB-13483, plugins-sdk-2.25.0 : Collector-2.0.0, AddOnForQt-3.9.1
1-Win7 Business Pack1 64bits : wx-3.2.4, gcc-8.1.0,
2-OpenSuse::Leap-15.4-64bits : wx-3.2.4;gtk3, gcc-8.2.1,
=> !! The messages are translated by Deepl