I'm completing my answer for the record to show exactly where your code is
faulty and wrong just like in the other previous cases.
1- you have a copy-paste pattern in your code
2- you are calling the kill command twice immediately one after another
without checking whether SIGTERM killed the process properly. The whole
purpose of doing SIGTERM is to allow cleaning resources before enforcing
SIGKILL. You're not giving the JVM an opportunity to call the shutdown hook
because you're looking up the same output from the same "ps ax" command for
both kill commands.
So your code is working almost exactly as if typing:
if (line ==~ /.*org\.apache\.ofbiz\.base\.start\.Start.*/) {
exec { commandLine 'kill', '-TERM', line.tokenize().first() }
exec { commandLine 'kill', '-KILL', line.tokenize().first() }
}
Now what's happening is that SIGKILL is executing before SIGTERM completes,
so the build is successful but OFBiz is killed with SIGKILL not SIGTERM. If
SIGTERM finishes very quickly (does not happen with me) then the build
system would crash because the exit value of SIGKILL would be 1, not 0
(process not found). So either way it is a problem (either killing with
SIGKILL or crashing the build system)
Convinced? I hope you can revert now.
On Sun, Jun 25, 2017 at 2:20 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:
> But what's wrong? It's not clear to me, do you see it Michael?
>
> Not clearly answering this question just adds more work on my side, I
> still don't see why I should revert.
>
> Jacques
>
>
>
> Le 25/06/2017 à 13:11, Michael Brohl a écrit :
>
>> I's like to propose the following process to get this worked out without
>> endless ping-pong here on the mailing lists:
>>
>> 1. revert the change as requested
>>
>> 2. provide a Jira with patch and review/discuss it there (what's the
>> problem, how should it be solved etc.)
>>
>> 3. come to a conclusion and a thumbsup by other committers
>>
>> 4. commit the solution.
>>
>> Thanks,
>>
>> Michael
>>
>>
>> Am 25.06.17 um 12:36 schrieb Taher Alkhateeb:
>>
>>> OK, I take this question as your refusal to revert and cooperate. I'm
>>> done
>>> spending time here and I leave it up to the community to see if they want
>>> incorrect code in the code base.
>>>
>>> On Sun, Jun 25, 2017 at 1:34 PM, Jacques Le Roux <
>>> jacques.le.r...@les7arts.com> wrote:
>>>
>>> Mmm but anyway, I remember now how I thought that.
>>>>
>>>> In the 2nd "if", if the process was terminated, kill will just say that
>>>> it
>>>> can't find the process and exec will return. Else the process will be
>>>> killed.
>>>>
>>>> What's wrong then?
>>>>
>>>> Jacques
>>>>
>>>>
>>>>
>>>> Le 25/06/2017 à 12:28, Jacques Le Roux a écrit :
>>>>
>>>> Ah wait, I see line still contains "start" so will be executed twice
>>>>> anyway. OK I'll improve that.
>>>>>
>>>>> Jacques
>>>>>
>>>>>
>>>>> Le 25/06/2017 à 12:18, Jacques Le Roux a écrit :
>>>>>
>>>>> I don't revert without explanations on why I should revert. Sorry, but
>>>>>> I
>>>>>> don't find your explanations clear.
>>>>>>
>>>>>> My explanation, tell my what's wrong:
>>>>>>
>>>>>> The 2 "if" blocks are executed sequentially for each line containing
>>>>>> "a
>>>>>> start"and ignore other lines. I did not change the previous logic,
>>>>>> just
>>>>>> added a new if.
>>>>>>
>>>>>> Gradle exec spawns a new process and waits till it ends (this point is
>>>>>> important).
>>>>>>
>>>>>> If the line contains "a start" the 1st "if" try to terminate it.
>>>>>>
>>>>>> If it worked the 2nd "if" does not execute. This is better than before
>>>>>> because it allows the "'start" process "a chance to clean up after
>>>>>> itself"
>>>>>> (cf unix.stackexchange.com below)
>>>>>>
>>>>>> If the "start" process is not terminated then it's killed by the 2nd
>>>>>> "if", like it was done before.
>>>>>>
>>>>>> As I said it cleanly worked with 2 OFBiz instances running in the
>>>>>> background.
>>>>>>
>>>>>> Now tell me what's wrong?
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>> Jacques
>>>>>>
>>>>>>
>>>>>> Le 25/06/2017 à 11:22, Taher Alkhateeb a écrit :
>>>>>>
>>>>>> As usual, you refuse to revert because you don't understand the code
>>>>>>> and I
>>>>>>> pay the price of spending my time explaining. I hope you will
>>>>>>> reconsider
>>>>>>> this time consuming and non-cooperative behavior.
>>>>>>>
>>>>>>> The quick version:
>>>>>>> - copy and paste antipattern
>>>>>>> - incorrect conditional checking leading to both blocks getting
>>>>>>> executed or
>>>>>>> both blocks not executing
>>>>>>>
>>>>>>> Your belief that Gradle fails because java does not expect to be
>>>>>>> killed
>>>>>>> is
>>>>>>> amazing! It means you do not understand what this code is doing and
>>>>>>> what is
>>>>>>> causing the failure.
>>>>>>>
>>>>>>>
>>>>>>> On Jun 25, 2017 10:42 AM, "Jacques Le Roux" <
>>>>>>> jacques.le.r...@les7arts.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>> What makes you think it's wrong? I tested it locally using 2
>>>>>>> background
>>>>>>> instances and it cleaned worked.
>>>>>>>
>>>>>>> I also tried with one standard instance (not in background). It
>>>>>>> works,
>>>>>>> and
>>>>>>> you get this message
>>>>>>>
>>>>>>> :ofbiz FAILED
>>>>>>> FAILURE: Build failed with an exception.
>>>>>>> * What went wrong:
>>>>>>> Execution failed for task ':ofbiz'.
>>>>>>>
>>>>>>> Process 'command '/usr/lib/jvm/java-8-oracle/bin/java'' finished
>>>>>>>> with
>>>>>>>>
>>>>>>>> non-zero exit value 137
>>>>>>>
>>>>>>> Which I believe is OK because Java does not expect to be killed!
>>>>>>>
>>>>>>> Jacques
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Le 24/06/2017 à 20:04, Taher Alkhateeb a écrit :
>>>>>>>
>>>>>>> This commit is wrong and bad on multiple levels. Please revert
>>>>>>>
>>>>>>>> On Sat, Jun 24, 2017 at 10:56 AM, <jler...@apache.org> wrote:
>>>>>>>>
>>>>>>>> Author: jleroux
>>>>>>>>
>>>>>>>> Date: Sat Jun 24 07:56:45 2017
>>>>>>>>> New Revision: 1799736
>>>>>>>>>
>>>>>>>>> URL: http://svn.apache.org/viewvc?rev=1799736&view=rev
>>>>>>>>> Log:
>>>>>>>>> No functional change
>>>>>>>>>
>>>>>>>>> Improves terminateOfbiz byt using TERM before KILL
>>>>>>>>> https://fr.wikipedia.org/wiki/Kill_(Unix)
>>>>>>>>> https://unix.stackexchange.com/questions/8916/when-
>>>>>>>>> should-i-not-kill-9-a-process
>>>>>>>>>
>>>>>>>>> Modified:
>>>>>>>>> ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>>>>>
>>>>>>>>> Modified: ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
>>>>>>>>> build.gradle?rev=1799736&r1=1799735&r2=1799736&view=diff
>>>>>>>>> ============================================================
>>>>>>>>> ==================
>>>>>>>>> --- ofbiz/ofbiz-framework/trunk/build.gradle (original)
>>>>>>>>> +++ ofbiz/ofbiz-framework/trunk/build.gradle Sat Jun 24 07:56:45
>>>>>>>>> 2017
>>>>>>>>> @@ -332,8 +332,13 @@ task terminateOfbiz(group: ofbizServer,
>>>>>>>>> standardOutput = processOutput
>>>>>>>>> }
>>>>>>>>> processOutput.toString().split(System.lineSeparator()).each
>>>>>>>>> { line ->
>>>>>>>>> + // Try to terminate cleanly
>>>>>>>>> if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>>>>>> tart\.Start.*/)
>>>>>>>>> {
>>>>>>>>> - exec { commandLine 'kill', '-9',
>>>>>>>>> line.tokenize().first() }
>>>>>>>>> + exec { commandLine 'kill', '-TERM',
>>>>>>>>> line.tokenize().first() }
>>>>>>>>> + }
>>>>>>>>> + // Only kill if needed
>>>>>>>>> + if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>>>>>> tart\.Start.*/)
>>>>>>>>> {
>>>>>>>>> + exec { commandLine 'kill', '-KILL',
>>>>>>>>> line.tokenize().first() }
>>>>>>>>> }
>>>>>>>>> }
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>
>>>>>
>>
>>
>
