Assess streaming filter support behavior in a memory constrained environment

[nulltoken]

Considering this #1030 (comment) and the comments below it, there are chances we may not play nice when the GC does its memory reclaiming duty.

It may interesting to test drive our implementation while the GC is running.

/cc @ammeep @shiftkey @ethomson @jamill @whoisj

[whoisj]

While I have a very hard time believing that the CLR, especially on Windows where it is the most mature, would incorrectly over collect objects and thus result in NullReferenceException I suppose it wouldn't hurt to understand how intensive garbage collection impacts the library.

It may interesting to test drive our implementation while the GC is running.

As I understand it, the GC is always running in the modern CLR. This allows it to be less impactful and helps to avoid the old "stuttering" that happened when the 1.0 CLR would suddenly decide to lock the object graph and start freeing memory.

I believe the real test here is how does the library perform under extreme memory pressure (example: 10 GB commit on a 8GB system).

Thoughts?

[carlosmn]

It seems even odder that if the GC did collect some objects, their references would be set to null as that would mean the GC went and found those references and decided to set them to null even though it thought there were no references left to the object.

[ethomson]

Yeah I'd also be a little bit surprised. Usually those references dangle and some well-typed exception is thrown.

I'm curious what* is null when this error occurs?

[whoisj]

It seems even odder that if the GC did collect some objects, their references would be set to null as that would mean the GC went and found those references and decided to set them to null even though it thought there were no references left to the object.

My thoughts exactly. I'll try and ask Jared Parsons if that is even possible, but I highly doubt it is.

I'm curious what* is null when this error occurs?

Me too. The reporting from the tests always accuse the first line of the test of being null. Perhaps it is a bug in xunit itself?

[whoisj]

Jared got back to me, and no the GC will not collect any object which have references. He points us to look at potential issues with global state, which I think is completely reasonable.

The filters are registered globally, as in the loading of Libgit2, etc.

[ammeep]

He points us to look at potential issues with global state, which I think is completely reasonable.

One thing I have seen locally (when using test driven .net) is if the app domain is not torn down then any corrupted global state (from failing filter test runs) would naturally cause test failures on subsequent test runs.

I'm not familiar with exactly how app veyor works under the hood - does it tear down the entire app domain on each test run?

[nulltoken]

I'm not familiar with exactly how app veyor works under the hood - does it tear down the entire app domain on each test run?

AFAIK, each AppVeyor job (ie. build matrix entry) is run on a brand new blank machine instance.

[shiftkey]

Would it be worth experimenting with GC settings to see if we can get any sort of consistency on this?

Also, knowing where the NRE is occurring would be amazing right now... 😭

[whoisj]

Fiddling with my Exception settings, I've managed to collect some details on the NRE which keep occurring on a number of the write-to-disk tests.

Test Name: LibGit2Sharp.Tests.FilterFixture.CanFilterLargeFiles
Test Outcome: Failed
Result Message: System.NullReferenceException : Object reference not set to an instance of an object.
Result StandardOutput:
The directory 'X:\LibGit2Sharp-TestRepos\x4ez4qkq.zdu' could not be deleted (5 attempts were made) due to a System.IO.IOException: The process cannot access the file '09c4dc44-f22d-4b10-ad3e-23f0a527389c.blob' because it is being used by another process.
Most of the time, this is due to an external process accessing the files in the temporary repositories created during the test runs, and keeping a handle on the directory, thus preventing the deletion of those files.
Known and common causes include:

• Windows Search Indexer (go to the Indexing Options, in the Windows Control Panel, and exclude the bin folder of LibGit2Sharp.Tests)
• Antivirus (exclude the bin folder of LibGit2Sharp.Tests from the paths scanned by your real-time antivirus)
• TortoiseGit (change the 'Icon Overlays' settings, e.g., adding the bin folder of LibGit2Sharp.Tests to 'Exclude paths' and appending an '*' to exclude all subfolders as well)

Looks like something is keeping a lock on the temporary files, the clean up tasks are failing and throwing - thus causing the next test in line to fail with the blame.

I'm confirmed that Windows Search Indexer and my corporate prescribed Antivirus is excluding the local X:\ drive completely; and I do not have Tortoise anything installed. Thus I do not believe any of the suggested items are to blame.

Is it possible that something in the utfx is keeping a handle on files and blocking the clean up operations?

[ethomson]

@whoisj Out of curiosity, what version of LibGit2Sharp are you seeing these problems with?

[nulltoken]

When a folder cannot be deleted, this usually point at "A handle hasn't been properly released" cause. Would that be the case, as the leak detection mechanism hasn't cringed, it may mean that this handle hasn't been converted to a SafeHandle (but marshaled as an IntPtr).

Being compelled to force-quit TestDriven.net usually point at the same cause.

[whoisj]

@ethomson top-of-tree (or very near the bleeding edge of vNext) as this is code I'm compiling and running local unit tests in Visual Studio.

@nulltoken there's code in the filter where IntPtr were used instead of SafeHandles due to convenience. Do you think this is the issue (at least for the FilterFixture issues)?

[nulltoken]

@whoisj How about opening a PR with the following changes:

• Drop most of the time consuming tests (to keep the build as short as possible)
• Decorate the Filter tests causing a NRE with some Trace.Writeline() calls (ie. à la printf debugging mode)
• Wrap those tests in loops in order to raise the chance to see those failing