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

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 11931
    • 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: 2454
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: 11931
    • 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: 11931
    • 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: 2454
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: 11931
    • 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!]

Offline BlueHazzard

  • Developer
  • Lives here!
  • *****
  • Posts: 2454
Re: Crash with batch build on linux
« Reply #36 on: September 09, 2019, 11:59:52 pm »
Ok here is a other version of this patch with hopefully improved formatting and comments.

I test this patch currently extensively on my build environment (windows and unix). If a can not find any bigger breaks, we should consider to merge this into trunk, and for this we have to decide who removes the wx28 projects....

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 11931
    • Travis build status
Re: Crash with batch build on linux
« Reply #37 on: September 10, 2019, 06:47:42 am »
Why have you changed the process stuff?
For now it is best to make this only compile if building against wx3.x.
We'll remove them when we remove wx2.8 support.
(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: 2454
Re: Crash with batch build on linux
« Reply #38 on: September 10, 2019, 08:15:04 am »
Quote
Why have you changed the process stuff?
Because i have an automated build system for linux and windows that builds itself and then builds codeblocks automatically. I can not test one patch without the other, because if the process stuff is missing, the build will stall on windows, and if the CallAfter part is missing the build will crash on linux.

Code: [Select]
For now it is best to make this only compile if building against wx3.x.Ok i will wrap it in #ifdefs

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 11931
    • Travis build status
Re: Crash with batch build on linux
« Reply #39 on: September 10, 2019, 06:17:02 pm »
Please separate them in two patches/commits.
The process stuff is really fragile, and when I say this I mean it. There is no way to test everything affected by this. :(

Also the flag parameter of the Launch method seems to be ignored. Aren't you getting unused variable warnings for this? Is there a reason to pass it at all? Comments why you need this?
(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: 2454
Re: Crash with batch build on linux
« Reply #40 on: September 12, 2019, 10:33:01 pm »
First part. Fix crash on linux

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 11931
    • Travis build status
Re: Crash with batch build on linux
« Reply #41 on: September 13, 2019, 08:20:07 am »
Why are you showing a message box in OnCloseBatchBuildWindow? It should be possible to run batch build on a headless machine where there is no operator. For example running in on appvoyer.
(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: 2454
Re: Crash with batch build on linux
« Reply #42 on: September 13, 2019, 09:33:29 pm »
Why are you showing a message box in OnCloseBatchBuildWindow? It should be possible to run batch build on a headless machine where there is no operator. For example running in on appvoyer.
We had this discussion on top (or at SF):
One step at the time
1) we fix that it can be used (because right now it simply crashes (on linux) and stalls (on windows). So it can't be used with or without dialog....
2) we remove all UI code from this part

at the moment we are at 1) . If the build runs (on linux and windows) we can move to step 2)

Is this patch ok for you?

Offline BlueHazzard

  • Developer
  • Lives here!
  • *****
  • Posts: 2454
Re: Crash with batch build on linux
« Reply #43 on: Yesterday at 12:28:29 am »
Here is the windows part. Thank you for showing me the error of the flags. I tried also to add some documentation and why we use this timer function...

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 11931
    • Travis build status
Re: Crash with batch build on linux
« Reply #44 on: Yesterday at 01:28:08 am »
(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!]