Opened 5 weeks ago

Closed 3 weeks ago

#22927 closed defect (fixed)

zstd tests fail with libzstd 1.3.0

Reported by: teor Owned by:
Priority: High Milestone: Tor: 0.3.1.x-final
Component: Core Tor/Tor Version: Tor: 0.3.1.1-alpha
Severity: Normal Keywords: tor-dir, review-group-21
Cc: ahf Actual Points: 1
Parent ID: Points: 1
Reviewer: Sponsor:

Description

When I run the unit tests on tor master:
`
[notice] Tor 0.3.2.0-alpha-dev (git-9a1338d9df938fba) running on Darwin with Libevent 2.1.8-stable, OpenSSL 1.0.2l, Zlib 1.2.11, Liblzma 5.2.3, and Libzstd 1.3.0.
`

I see the following failures:

buffer/compress/zstd: [forking] 
  FAIL src/test/test_buffers.c:607: assert(write_to_buf_compress(buf, compress_state, "", 0, 1) OP_EQ 0): -1 vs 0
  FAIL src/test/test_buffers.c:607: assert(write_to_buf_compress(buf, compress_state, "", 0, 1) OP_EQ 0): -1 vs 0
  FAIL src/test/test_buffers.c:607: assert(write_to_buf_compress(buf, compress_state, "", 0, 1) OP_EQ 0): -1 vs 0
  FAIL src/test/test_buffers.c:607: assert(write_to_buf_compress(buf, compress_state, "", 0, 1) OP_EQ 0): -1 vs 0
  [compress/zstd FAILED]
...
util/compress_concat/zstd: 
  FAIL src/test/test_util.c:2414: assert(r OP_EQ 0): -1 vs 0
  [compress_concat/zstd FAILED]

Child Tickets

Attachments (1)

zstd-test.c (1.5 KB) - added by nickm 5 weeks ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 5 weeks ago by nickm

Priority: MediumHigh

Can reproduce.

I wonder if this has anything to do with zstd 1.3.0, which just came out a few days ago?

comment:2 Changed 5 weeks ago by nickm

When I run those tests with the --info option I see a bunch of "Zstandard compression unable to write epilogue: Context should be init first"

comment:3 Changed 5 weeks ago by nickm

Cc: ahf added

comment:4 in reply to:  1 Changed 5 weeks ago by teor

Replying to nickm:

Can reproduce.

I wonder if this has anything to do with zstd 1.3.0, which just came out a few days ago?

I only saw them after I upgraded.

comment:5 Changed 5 weeks ago by nickm

Based on some examination, here is what I think is happening: we are done compressing our input, so we call ZSTD_endStream to write the epilogue... but the output buffer is full, so we have to grow the output buffer and try again... so we try ZSTD_endStream again, but it doesn't work for some reason.

comment:6 Changed 5 weeks ago by nickm

(The documentation implies that this should work, though:

*  ZSTD_endStream() instructs to finish a frame.
*  It will perform a flush and write frame epilogue.
*  The epilogue is required for decoders to consider a frame completed.
*  Similar to ZSTD_flushStream(), it may not be able to flush the full content i
f `output->size` is too small.
*  In which case, call again ZSTD_endStream() to complete the flush.
*  @return : nb of bytes still present within internal buffer (0 if it's empty, 
hence compression completed)
*            or an error code, which can be tested using ZSTD_isError().

comment:7 Changed 5 weeks ago by teor

The latest docs have the following extra lines:

  @return : 0 if frame fully completed and fully flushed,
             or >0 if some data is still present within internal buffer
                  (value is minimum size estimation for remaining data to flush, but it could be more)
            or an error code, which can be tested using ZSTD_isError().

typedef ZSTD_CCtx ZSTD_CStream;  /**< CCtx and CStream are now effectively same object (>= v1.3.0) */

https://facebook.github.io/zstd/zstd_manual.html

comment:8 Changed 5 weeks ago by nickm

I think I have it figured out -- the problem is that we have an intervening call to zstd_compressstream.

I'm attaching a program that succeeds under zstd 1.2.0, but fails under zstd 1.3.0.

Changed 5 weeks ago by nickm

Attachment: zstd-test.c added

comment:9 Changed 5 weeks ago by nickm

Summary: zstd tests fail on masterzstd tests fail with libzstd 1.3.0

comment:10 Changed 5 weeks ago by nickm

Actual Points: 1
Status: newneeds_review

See branch bug22927_031 in my public repository. It seems to fix the issue for me.

Ahf, if you have a chance, could you look over this code?

comment:11 Changed 5 weeks ago by nickm

Keywords: review-group-21 added

comment:12 Changed 3 weeks ago by ahf

Status: needs_reviewmerge_ready

LGTM.

comment:13 Changed 3 weeks ago by nickm

Resolution: fixed
Status: merge_readyclosed

thanks; merged to 0.3.1 and forward.

Note: See TracTickets for help on using tickets.