Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

halt_error/1 crashes when called more than once #2358

Closed
pkoppstein opened this issue Oct 15, 2021 · 4 comments
Closed

halt_error/1 crashes when called more than once #2358

pkoppstein opened this issue Oct 15, 2021 · 4 comments
Labels
Milestone

Comments

@pkoppstein
Copy link
Contributor

The bash script output below shows the jq version numbers and the crash report:

function json {
cat <<EOF
{"err": 1}
{}
{}
{"err": 1}
EOF
}

for jq in jq1.6 jqMaster
do
  $jq --version
  json | $jq -r '
    if(.err==null) then 0
    else "Failed to process " | halt_error(1) 
    end'
  echo
done

Output:

jq-1.6
Failed to process 0
0
Failed to process jq(55072,0x110bef5c0) malloc: *** error for object 0x7fcc38c03d70: pointer being freed was not allocated
jq(55072,0x110bef5c0) malloc: *** set a breakpoint in malloc_error_break to debug
./halt_error.bug: line 26: 55071 Done                    json
     55072 Abort trap: 6           | $jq -r '
  if(.err==null) 
  then 0
  else "Failed to process " | halt_error(1) 
  end'

jq-1.6-129-g80052e5-dirty
jq: error: Failed to process 0
0
jqMaster(55076,0x1100485c0) malloc: *** error for object 0x3: pointer being freed was not allocated
jqMaster(55076,0x1100485c0) malloc: *** set a breakpoint in malloc_error_break to debug
./halt_error.bug: line 26: 55075 Done                    json
     55076 Abort trap: 6           | $jq -r '
  if(.err==null) 
  then 0
  else "Failed to process " | halt_error(1) 
  end'

Environment: Mac OS

@itchyny itchyny added the bug label Jun 3, 2023
@itchyny itchyny added this to the 1.7 release milestone Jun 25, 2023
@itchyny
Copy link
Contributor

itchyny commented Jun 27, 2023

I cannot reproduce the issue on macOS 13.4.1.

@emanuele6
Copy link
Member

emanuele6 commented Jun 27, 2023

On my Linux 6.1.35-lts, I get no crash for jq master:

$ json | ./jq -r '
    if(.err==null) then 0
    else "Failed to process " | halt_error(1) 
    end'
Failed to process 0
0
Failed to process 

But I get a crash for jq 1.6: (note, also the 0s were coloured as numbers in jq master, but not in jq 1.6)

$ json | jq -r '
    if(.err==null) then 0
    else "Failed to process " | halt_error(1) 
    end'
Failed to process 0
0
Failed to process malloc_consolidate(): unaligned fastbin chunk detected
Aborted (core dumped)

So it must have been fixed.


But I think there is still a behaviour problem, because I think halt_error(1) is supposed to exit immediately with exit status 1 instead of going to the next input

I don't think it should be possible to "call halt_error more than once"

$ json | jqExpectedBehaviour -r '
    if(.err==null) then 0
    else "Failed to process " | halt_error(1) 
    end'
Failed to process 
$ echo "$?"
1

@emanuele6
Copy link
Member

emanuele6 commented Jun 27, 2023

But I think there is still a behaviour problem, because I think halt_error(1) is supposed to exit immediately with exit status 1 instead of going to the next input

This patch would fix that:

diff --git a/src/main.c b/src/main.c
index 1cc6542..462c62a 100644
--- a/src/main.c
+++ b/src/main.c
@@ -681,6 +681,8 @@ int main(int argc, char* argv[]) {
         ret = process(jq, value, jq_flags, dumpopts);
         if (ret <= 0 && ret != JQ_OK_NO_OUTPUT)
           last_result = (ret != JQ_OK_NULL_KIND);
+        if (jq_halted(jq))
+          break;
         continue;
       }
 
bash-5.1$ # jq 1.6
bash-5.1$ <<< '{} {}' jq '1, ("halt!\n" | halt_error(10)), 3'; printf 'ret: %s\n' "$?"
1
halt!
1
halt!
ret: 10
bash-5.1$ # jq patched
bash-5.1$ <<< '{} {}' ./jq '1, ("halt!\n" | halt_error(10)), 3'; printf 'ret: %s\n' "$?"
1
halt!
ret: 10

Or maybe it's just the man page that is worded incorrectly:

   halt_error, halt_error(exit_code)
       Stops the jq program with no further outputs. The input will be printed
       on stderr as raw output (i.e., strings will  not  have  double  quotes)
       with no decoration, not even a newline.

       The given exit_code (defaulting to 5) will be jq´s exit status.

       For example, "Error: somthing went wrong\n"|halt_error(1).

From the man page, it sounds like the program should stop as it does with my patch, and that is how i always expected it work.

gojq behaves like jq on master; and fq behaves like my patched version:

bash-5.1$ <<< '{} {}' gojq '1, ("halt!\n" | halt_error(10)), 3'; printf 'ret: %s\n' "$?"
1
halt!
1
halt!
ret: 10
bash-5.1$ <<< '{} {}' fq ...

bash-5.1$ <<< '{} {}' fq '1, ("halt!\n" | halt_error(10)), 3'; printf 'ret: %s\n' "$?"
1
error: halt!

ret: 10

I think the correct behaviour is that the entire program stops like the documentation says, and that requiring users to do jq -n 'inputs | ... | halt_error(1)' to make halt_error actually stop the entire program is an unnecessary annoyance, but since gojq and jq master works this way, maybe we could just change the documenation if we prefer.

EDIT: fq only works that way because it implicitly slurps if there are multiple elements in the inputs, so the code is only ran once with [{},{}].

@itchyny
Copy link
Contributor

itchyny commented Jun 30, 2023

The reported bug seems to be fixed by cf4b48c. Not sure where in the patch is related, but I double checked that cf4b48c~1 crashes but cf4b48c does not. Since the original bug is already fixed, closing this issue.

@emanuele6 Good find. I agree the behavior is confusing, but I think this is an intended behavior. And already reported by #2368.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants