Skip to content

Fix crash when running multiple filter smudge or clean operations concurrently #1260

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

Merged
merged 2 commits into from
Feb 18, 2016

Conversation

niik
Copy link
Contributor

@niik niik commented Feb 17, 2016

The current filter implementation in libgit2sharp stores the input/output stream pointers and more as instance variables on the Filter class. Unfortunately this makes it impossible to run multiple filter streams concurrently.

Due to the nature of the problem manifests itself in a few different ways, sometimes as an unmanaged crash due to invalid pointer access, sometimes as a managed exception. When run under the debugger you'll sometimes get notified that a callback has been collected.

CallbackOnCollectedDelegate occurred
Message: Managed Debugging Assistant 'CallbackOnCollectedDelegate' has detected a problem in 'Foo.Exe'.
Additional information: A callback was made on a garbage collected delegate of type 'LibGit2Sharp!LibGit2Sharp.Core.GitWriteStream+close_fn::Invoke'. This may cause application crashes, corruption and data loss. When passing delegates to unmanaged code, they must be kept alive by the managed application until it is guaranteed that they will never be called.

This PR changes the implementation to root the necessary references in a dictionary keyed by the stream pointer.

cc @carlosmn @ethomson

@shiftkey
Copy link
Contributor

@carlosmn
Copy link
Member

Looks good, could you squash the commits after f7e50b2 into ti? We try to keep the history free of small commits fixing up those from before in a PR.

@niik niik force-pushed the concurrent-filters branch from 5d36ff5 to 6da1012 Compare February 18, 2016 12:25
niik added 2 commits February 18, 2016 13:26
The current implementation of filters in libgit2sharp only allows for
one filter stream per type of filter at any given point in time. This
switches the storage of the necessary state from instance variables on
the Filter class to a dedicated state object which is tracked in a
dictionary keyed on the libgit2 stream pointer.
@niik niik force-pushed the concurrent-filters branch from 6da1012 to 1ba31db Compare February 18, 2016 12:27
@niik
Copy link
Contributor Author

niik commented Feb 18, 2016

🐨

carlosmn added a commit that referenced this pull request Feb 18, 2016
Fix crash when running multiple filter smudge or clean operations concurrently
@carlosmn carlosmn merged commit 92eaebc into libgit2:vNext Feb 18, 2016
@carlosmn
Copy link
Member

🐻

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.

3 participants