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

Fix implementation of ThrottledStream.Write #3811

Merged
merged 6 commits into from Jul 9, 2019

Conversation

@warwickmm
Copy link
Contributor

warwickmm commented Jul 8, 2019

This fixes an issue with the ThrottledStream.Write method, where the offset into the source buffer was not advanced properly. Prior to revision 0190a99, the DelayIfRequired method would increment the offset variable to advance through the buffer. When we modified the implementation in revision 0190a99, we may have forgotten to modify the offset accordingly.

We also add unit tests for ThrottledStream.Write as well as ThrottedStream.Read (see issue #3787, which was fixed in pull request #3802).

This might fix issue #3782.

warwickmm added 5 commits Jul 7, 2019
Prior to revision 0190a99 ("Rewrote the throttling implementation to
be more effecient"), the DelayIfRequired method would increment the
offset variable to advance through the buffer.  When we modified the
implementation in revision 0190a99, we may have forgotten to modify
the offset accordingly.
This concerns issue #3787, which was fixed in pull request #3802.
/// </summary>
/// <param name="buffer">The buffer to write to.</param>
/// <param name="buffer">The buffer to read from.</param>
/// <param name="offset">The offset into the buffer.</param>
/// <param name="count">The number of bytes to write.</param>
public override void Write(byte[] buffer, int offset, int count)

This comment has been minimized.

Copy link
@warwickmm

warwickmm Jul 8, 2019

Author Contributor

This method uses a mixture of spaces and tabs for the indentation. I decided to not fix the indentation to keep the diff here cleaner looking.


[Test]
[Category("Utility")]
public static void ThrottledStreamRead()

This comment has been minimized.

Copy link
@warwickmm

warwickmm Jul 8, 2019

Author Contributor

This test fails without the changes from pull request #3802.


[Test]
[Category("Utility")]
public static void ThrottledStreamWrite()

This comment has been minimized.

Copy link
@warwickmm

warwickmm Jul 8, 2019

Author Contributor

This test fails without the changes from revision 5d08779.

@Pectojin Pectojin merged commit 1ad2f35 into duplicati:master Jul 9, 2019
3 checks passed
3 checks passed
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@warwickmm warwickmm deleted the warwickmm:fix_throttled_write branch Jul 10, 2019
@duplicatibot

This comment has been minimized.

Copy link

duplicatibot commented Jul 17, 2019

This pull request has been mentioned on Duplicati. There might be relevant details there:

https://forum.duplicati.com/t/restore-failing-on-3-seperate-servers-on-b2/6780/22

@duplicatibot

This comment has been minimized.

Copy link

duplicatibot commented Nov 15, 2019

This pull request has been mentioned on Duplicati. There might be relevant details there:

https://forum.duplicati.com/t/failed-invalid-header-marker/2871/21

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.