#854 Batch build stalls when window is not active

[bluehazzard]

As reported here: http://forums.codeblocks.org/index.php/topic,22824.msg155493.html#msg155493
the batch build process will stall if the window is not active and a lot text is outputed

[bluehazzard]

Can please someone test this on linux in a heavy build environment? I will try to test it in the next time on my build server, but it would be good if someone else tests this too.

this is really needed on windows, and it fixes the problem as reported in the forum and also by my experience

[Morten MacFly]

Is there any news on this? This issue really annoys me for years on Windows.
On Windows, it seems to work again with this patch. The only thnig I am not able to find out is why we initially commented these two lines with the timer. This should be "protocolled" somewhere in the SVN history.

[bluehazzard]

The commit is described here:
http://forums.codeblocks.org/index.php/topic,22824.msg155247.html#msg155247

It is the commit with the number 5 :) . Sadly there is no information, why this is commented out...

The status is, that we have a working patch, that fixes the issue, but some styling is missing (removing default parameter) I will do this tomorrow and commit it.... Then is a 15 year old bug fixed...

[bluehazzard]

On a second thought, there is the possibility that there is a performance penalty... (creating a timer for every process is probably not necessary)... I made no measurements.... but i think there is space for improvements... For example one timer for all processes... Can i use an static variable here?

[bluehazzard]

I have now tried to move this to one timer.... Until now no success...

[Morten MacFly]

Will this get comitted at some point? And I think the bug is not there since 15 years, but definitely shorter. Meanwhile maybe 2-4 years (time goes by so fast), it used to work w/o stalls that time, too. One could try to bisect when the stall got introduced... but it will take a lot time...

[bluehazzard]

I am really sorry, but i have absolute no time for probably the next 2 weeks to do anything....

[Andrew Cottrell]

In my testing building Code::Blocks does not cause the stall. Attached are CPU graphs for the following builds:
- My latest code without the patch
- My latest code with the patch
- The Nightly 12487 release

From the graphs I captured it looks like build is faster with the patch, bu not significantly from the graph.

Next stop is to grab the example git repo and try it to see if I get a stall.

[Andrew Cottrell]

It looks like if I minmise the C:B window then after a while the stall occurs. I checked the CPU after posting the post above and spotted the stall.

Stall CPU graph attached.

[Andrew Cottrell]

The stall in the previous post was with the nightly build SVN12487.

I have not been able to create another stall. I tried the fake GCC forum link, but the C:B dialog did not even show. I am testing with the MSY2 MinGW GCC 10.2 compiler.

I would make two slight mods to the patch as follows:
1) Move the m_timerPollProcess.SetOwner(). to the PipedProcess::PipedProcess constructor so it is only setup once to ensure it is not called twice just in case the underlying library has issues calling it multiple times if someone mods the code in the future.
2) Add the following two lines to the destructor for another just in case scenario. This ensures that timer stops when the class destrutor is called instead of leaving it up to the underlying timer library to do the cleanup as I was burnt with this way way back in the MS VC++ 6 days.

     if (m_timerPollProcess.IsRunning())
        m_timerPollProcess.Stop();

I have not had any issues building C:B on windows with the fix in the code.

[bluehazzard]

Thank you for testing and your comments on the code.
this patch is in revision: r12494

[Teodor Petrov]

@bluehazzard:
1. Any time you change the API/ABI in the include folder you're supposed to rise PLUGIN_SDK_VERSION_MINOR by one
2. What is going on with this flag parameter? - Morten has changed it in to cb_unused in the following commit [r12495], but you've added it in [r12494] one of these commit is wrong - either remove the parameter, or make it do something useful!

[Teodor Petrov]

1. Adding a default parameter to the same place where there is another default parameter, and both parameters being the same type is a disaster waiting to happen. Don't do it. Generally don't do default parameters at all.
2. Flag parameters must be unsigned type and most of the time it is best to use something with known and fixed size. Here I guess you're replicating the error wxWidgets people did, I guess.

[bluehazzard]

You are right, the flags parameter should not be unused, because (at least on windows) it is needed.
I fixed this in svn
I have also removed the poll timer interval parameter, because it is not needed, i think a fixed value is ok for this...

About enum and flags, well i think it is better to use the same type as wxExecute expects or not?