Skip to content

Commit

Permalink
Fix parsing deadlock (#57)
Browse files Browse the repository at this point in the history
Parser will deadlock if hitting `if len(pj.containingScopeOffset) != 0` before returning, since 
the stream is done, but we return false.

Return status and done status separately.
  • Loading branch information
klauspost authored Feb 22, 2022
1 parent 68d6648 commit 9743702
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 11 deletions.
12 changes: 7 additions & 5 deletions parse_json_amd64.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,14 @@ func (pj *internalParsedJson) parseMessage(msg []byte, ndjson bool) (err error)
wg.Add(1)
go func() {
defer wg.Done()
if !pj.unifiedMachine() {
if ok, done := pj.unifiedMachine(); !ok {
err = errors.New("Bad parsing while executing stage 2")
// Keep consuming...
for idx := range pj.indexChans {
if idx.index == -1 {
break
if !done {
for idx := range pj.indexChans {
if idx.index == -1 {
break
}
}
}
}
Expand All @@ -101,7 +103,7 @@ func (pj *internalParsedJson) parseMessage(msg []byte, ndjson bool) (err error)
}
return errors.New("Failed to find all structural indices for stage 1")
}
if !pj.unifiedMachine() {
if ok, _ := pj.unifiedMachine(); !ok {
// drain the channel until empty
for {
select {
Expand Down
7 changes: 6 additions & 1 deletion parse_json_amd64_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -637,26 +637,31 @@ func benchmarkParseNumber(b *testing.B, neg int) {
}

func BenchmarkParseNumberFloat(b *testing.B) {
b.SetBytes(1)
for i := 0; i < b.N; i++ {
parseNumber([]byte("339.7784:"))
}
}

func BenchmarkParseAtof64FloatGolang(b *testing.B) {
b.SetBytes(1)
for i := 0; i < b.N; i++ {
strconv.ParseFloat("339.7784", 64)
}
}

func BenchmarkParseNumberFloatExp(b *testing.B) {
b.SetBytes(1)
for i := 0; i < b.N; i++ {
parseNumber([]byte("-5.09e75:"))
}
}

func BenchmarkParseNumberBig(b *testing.B) {
b.SetBytes(1)
x := []byte("123456789123456789123456789:")
for i := 0; i < b.N; i++ {
parseNumber([]byte("123456789123456789123456789:"))
parseNumber(x)
}
}

Expand Down
9 changes: 4 additions & 5 deletions stage2_build_tape_amd64.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,10 @@ func isValidNullAtom(buf []byte) bool {
return false
}

func (pj *internalParsedJson) unifiedMachine() bool {
func (pj *internalParsedJson) unifiedMachine() (ok, done bool) {
buf := pj.Message
const addOneForRoot = 1

done := false
idx := ^uint64(0) // location of the structural character in the input (buf)
offset := uint64(0) // used to contain last element of containing_scope_offset

Expand Down Expand Up @@ -433,17 +432,17 @@ succeed:

// Sanity checks
if len(pj.containingScopeOffset) != 0 {
return false
return false, done
}

pj.annotate_previousloc(offset>>retAddressShift, pj.get_current_loc()+addOneForRoot)
pj.write_tape(offset>>retAddressShift, 'r') // r is root

pj.isvalid = true
return true
return true, done

fail:
return false
return false, done
}

// structural chars here are
Expand Down

0 comments on commit 9743702

Please sign in to comment.