Compression errors with zlib 1.2.11

Bug #1656050 reported by maksis on 2017-01-12
14
This bug affects 1 person
Affects Status Importance Assigned to Milestone
AirDC++
Undecided
Unassigned
ApexDC++
Undecided
Unassigned
DC++
High
Unassigned

Bug Description

After updating zlib to version 1.2.10, transfers randomly fail because of compression errors. Such errors happening on uploader's side won't reported anywhere, the upload just gets disconnected.

Based on some debugging with AirDC++, the actual location where the error happens in zlib is https://github.com/madler/zlib/blob/master/deflate.c#L595

I'm not sure whether that's a bug in zlib or something intended as there are also breaking changes in version 1.2.10: https://github.com/madler/zlib/issues/190 (avail_out is not originally zero when the call is made)

Setting compression level at https://sourceforge.net/p/dcplusplus/code/ci/default/tree/dcpp/ZUtils.cpp#l59 to a non-zero value seems to help with avoiding the issue.

eMTee (realprogger) wrote :

I can confirm similar problems sporadically appearing during transfers between DC++ 0.864 and clients equipped with both zlib 1.2.10 and older versions.
Tested with downloading filelists, always the same transfers with same clients fail while the rest succeed so it probably depends on the data being compressed.

Changed in dcplusplus:
status: New → Confirmed
importance: Undecided → High
maksis (maksis) wrote :

I haven't experienced any errors with zlib 1.2.11 yet

eMTee (realprogger) wrote :

Nor have I so far.

Changed in dcplusplus:
status: Confirmed → Fix Committed
eMTee (realprogger) wrote :

Until multiple people tried and failed to get my current filelist today. Which is not working so long as I have the compression enabled (with zlib 1.2.11).

Changed in dcplusplus:
status: Fix Committed → Confirmed
summary: - Compression errors with zlib 1.2.10
+ Compression errors with zlib 1.2.10 & 1.2.11
eMTee (realprogger) on 2017-01-17
summary: - Compression errors with zlib 1.2.10 & 1.2.11
+ Compression errors with zlib 1.2.10
maksis (maksis) wrote :

Yeah, it still happens with files as well. The next stable version AirDC++ will be released with zlib 1.2.8 regardless of what happens with this issue.

Changed in airdcpp:
status: New → Confirmed
poy (poy) wrote :

patch that might work, cooked up by following the deflateParams doc. please test.

maksis (maksis) wrote :

No errors with a previously failing folder transfer after applying that patch

eMTee (realprogger) wrote :

No errors getting the same filelist from me which still fails without this patch.

maksis (maksis) wrote :

Should the check for "zs.avail_out == 0" still be kept in case zlib's error policy is changed again in future and for compatibility with older zlib versions? The code can't really be tied to a specific zlib version on Linux.

Shouldn't a split like that be done using the preprocessor... unless you are absolutely certain that retaining that condition with the new version has no adverse effects.

zlib has the necessary constants to do this and the patch is small enough to make it a viable approach.

maksis (maksis) wrote :

That's one way to do it.

For anyone using the code on Linux, it's very much impossible to notice that such split is needed (before they actually run into problems and try to figure out what's wrong). I'd strongly avoid such caveats that rely on internal functionality changes of a library instead of actual compiler errors.

maksis (maksis) on 2017-01-25
summary: - Compression errors with zlib 1.2.10
+ Compression errors with zlib 1.2.11
poy (poy) wrote :

this is actually easy to reproduce once one understands the causes. :)

pushed a test case that simulates compression being dynamically disabled, along with the fix I attached here (but with proper guards based on the zlib version).

Changed in dcplusplus:
status: Confirmed → Fix Committed
Changed in apexdc:
status: New → Fix Committed
maksis (maksis) on 2017-02-01
Changed in airdcpp:
status: Confirmed → Fix Released
poy (poy) wrote :

Fixed in DC++ 0.865.

Changed in dcplusplus:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers