Author Topic: Crash with batch build on linux  (Read 1797 times)

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 11899
    • Travis build status
Re: Crash with batch build on linux
« Reply #30 on: August 08, 2019, 11:10:32 pm »
@oBFusCATed: I don't think idle events are the way to go, they happen all the time and are not suited for one time tasks. These lambdas are nice to write code in one place and let it execute in another place, like work units for a work queue. But i also like auto so im afraid we won't agree here as well  ::) ;D
I know people like to run their whole applications from inside a lamdba, but this doesn't make it a good idea. Same as the almost-always-auto crap. I've seen it in the wild and it is not fun when something breaks. 8)
(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: 2432
Re: Crash with batch build on linux
« Reply #31 on: August 14, 2019, 12:16:02 am »
Ok, here is a cleaned up patch.
I think the limited use of lambda is ok in this context. I do not think that using the idle event is the right place. I think CallAfter is exactly the right way to do this.
i tested it on linux and windows. More testing is still needed.

This breaks compiling for wx2.8, but we wanted to remove it anyway :)

Have we a plan for removing wx2.8? Just removing projects and removing conditional code as can be found, or heavy code cleanup the next 6 months?

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 11899
    • Travis build status
Re: Crash with batch build on linux
« Reply #32 on: August 14, 2019, 08:08:18 am »
I'd just remove projects first. Then after a while I'll do a full cleanup of conditional code.
(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 oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 11899
    • Travis build status
Re: Crash with batch build on linux
« Reply #33 on: August 14, 2019, 08:12:40 am »
Formatting of lambda's should be either:

Code: [Select]
CallAfter([=]() {

});

or
Code: [Select]
CallAfter([=]()
{

});

I don't think what you're doing is a good idea. And please don't use the all-things capture modes. Capture specific variables. And please triple check pointer/reference captures would be 100% alive/valid at the time of the execution of the lambda.

You patch has zero comments explaining why you have to use these CallAfters. This is really unacceptable!

p.s. Also limit long lines to 100 or 120 chars.
« Last Edit: August 14, 2019, 08:14:37 am by oBFusCATed »
(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: 2432
Re: Crash with batch build on linux
« Reply #34 on: August 14, 2019, 09:35:22 am »
Quote
I don't think what you're doing is a good idea
Why not? The problem is we start the build process in the OnInit() method, and stop it in a codeblocks event method. In both cases the Output windows is destroyed before all window work is done.
With call after we move the heavy work functions to outside the OnInit and other Event method, when all work is done and the windows are ready to be updated or destroyed. This function only are called once: We start the build process only one time. No need to put this in the Idle event, that is called repeatedly, and we would need to introduce a complex state machine to check if this is the first call ecc... The same for closing the window. We can not close the windows in the build finished event, because the compiler plugin will use the output window after the event. But if we queue it in the event loop of wxWidgets it will get called after all other functions are finished. The right place to destroy the window....
If you have a better idea, you are welcome to provide it. I have read the event handling on wxWidgets and i do not see a counter point to this approach...

Quote
You patch has zero comments explaining why you have to use these CallAfters. This is really unacceptable!
Code: [Select]
diff --git a/src/src/app.cpp b/src/src/app.cpp
index 22a46dfb1..b6cc11fd3 100644
--- a/src/src/app.cpp
+++ b/src/src/app.cpp
@@ -749,8 +749,12 @@ bool CodeBlocksApp::OnInit()
             s_Loading = false;
             LoadDelayedFiles(frame);
 
-            BatchJob();
-            frame->Close();
+            // Start the batch build after finishing the init process
+            CallAfter([this]()
+                {
+                    this->BatchJob();
+                });
+
             return true;
         }

The other comments are noted and will be added

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 11899
    • Travis build status
Re: Crash with batch build on linux
« Reply #35 on: August 14, 2019, 07:12:55 pm »
Quote
I don't think what you're doing is a good idea
Why not?
I'm talking strictly about formatting here.

// Start the batch build after finishing the init process
Sorry, but this is useless comment. It is the same as commenting that you are setting a variable to some value. Good comments answer the why question and not the what question. (sometimes there is a need for a comment answering the what question, but this is not one of them).
(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!]