Skip to content

Conversation

@mandel-macaque
Copy link
Contributor

@mandel-macaque mandel-macaque commented Apr 15, 2019

The changes in mono are changing threading. This means that the code is
getting deadlock in the ReadStreamData.

This happens because we were not removing the handlers from the first request events.
Initially, before we had to take care about the auth headers, this was needed to init
certain internal state for the HttpContent that was exposed to the
user. With the fix of the CVE, the first request, in case of a redirect,
is garbage and is ignored. The second request is the one returned to the
user and contains all the required info.

Removing the event handlers, ensure that HandleCloseEvent is not called and
therefore ReadStream is never called.

The changes in mono are changing threading. This means that the code is
getting deadlock in the ReadStreamData.

This happens because we were not removing the handlers from the first requests events.
Initially, before we had to take care about the auth headers, this was needed to init
certain internal state for the HttpContent that was sexposed to the
user. With the fix of the CVE, the first request, in case of a redirect,
is garbage and is ignored. The second request is the one returned to the
user and contains all the required info.

Removing the event handlers, ensure that HandleClose is not called and
therefore ReadStream is never called.
@monojenkins
Copy link
Collaborator

Build success
Build succeeded
API Diff (from stable)
API Diff (from PR only) (no change)
Generator Diff (no change)
Test run succeeded

@akoeplinger
Copy link
Member

I ran the monotouch-tests on simulator on two macs, and on my iPad. All passed 👍

@mandel-macaque mandel-macaque merged commit c734cf2 into dotnet:mono-2019-02 Apr 16, 2019
@mandel-macaque mandel-macaque deleted the fix-cfnetwork-handler-timeout branch April 16, 2019 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants