PhysicalFileProvider: Avoid NullRef if ResolveLinkTarget returns null by jozkee · Pull Request #57235 · dotnet/runtime

jozkee · 2021-08-11T20:40:11Z

Spotted the following exception in #57229 (helix log)

Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.Extensions.FileProviders.Physical.FileSystemInfoHelper.GetFileLinkTargetLastWriteTimeUtc(FileInfo fileInfo) in /_/src/libraries/Microsoft.Extensions.FileProviders.Physical/src/Internal/FileSystemInfoHelper.cs:line 57
   at Microsoft.Extensions.FileProviders.Physical.PollingFileChangeToken.GetLastWriteTimeUtc() in /_/src/libraries/Microsoft.Extensions.FileProviders.Physical/src/PollingFileChangeToken.cs:line 57
   at Microsoft.Extensions.FileProviders.Physical.PollingFileChangeToken.get_HasChanged() in /_/src/libraries/Microsoft.Extensions.FileProviders.Physical/src/PollingFileChangeToken.cs:line 101
   at Microsoft.Extensions.FileProviders.Physical.PhysicalFilesWatcher.RaiseChangeEvents(Object state) in /_/src/libraries/Microsoft.Extensions.FileProviders.Physical/src/PhysicalFilesWatcher.cs:line 455
   at System.Threading.TimerQueueTimer.Fire(Boolean isThreadPool) in /_/src/libraries/System.Private.CoreLib/src/System/Threading/Timer.cs:line 673
   at System.Threading.TimerQueue.FireNextTimers() in /_/src/libraries/System.Private.CoreLib/src/System/Threading/Timer.cs:line 326

ghost · 2021-08-11T20:40:15Z

Tagging subscribers to this area: @maryamariyan, @dotnet/area-extensions-filesystem
See info in area-owners.md if you want to be subscribed.

Issue Details

Spotted the following exception in #57229 (helix log)

Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.Extensions.FileProviders.Physical.FileSystemInfoHelper.GetFileLinkTargetLastWriteTimeUtc(FileInfo fileInfo) in /_/src/libraries/Microsoft.Extensions.FileProviders.Physical/src/Internal/FileSystemInfoHelper.cs:line 57
   at Microsoft.Extensions.FileProviders.Physical.PollingFileChangeToken.GetLastWriteTimeUtc() in /_/src/libraries/Microsoft.Extensions.FileProviders.Physical/src/PollingFileChangeToken.cs:line 57
   at Microsoft.Extensions.FileProviders.Physical.PollingFileChangeToken.get_HasChanged() in /_/src/libraries/Microsoft.Extensions.FileProviders.Physical/src/PollingFileChangeToken.cs:line 101
   at Microsoft.Extensions.FileProviders.Physical.PhysicalFilesWatcher.RaiseChangeEvents(Object state) in /_/src/libraries/Microsoft.Extensions.FileProviders.Physical/src/PhysicalFilesWatcher.cs:line 455
   at System.Threading.TimerQueueTimer.Fire(Boolean isThreadPool) in /_/src/libraries/System.Private.CoreLib/src/System/Threading/Timer.cs:line 673
   at System.Threading.TimerQueue.FireNextTimers() in /_/src/libraries/System.Private.CoreLib/src/System/Threading/Timer.cs:line 326

<table>
  <tr>
    <th align="left">Author:</th>
    <td>Jozkee</td>
  </tr>
  <tr>
    <th align="left">Assignees:</th>
    <td>Jozkee</td>
  </tr>
  <tr>
    <th align="left">Labels:</th>
    <td>

`area-Extensions-FileSystem`

</td>
  </tr>
  <tr>
    <th align="left">Milestone:</th>
    <td>6.0.0</td>
  </tr>
</table>
</details>

carlossanlop

LGTM

danmoseley

Wondering whether this is unit testable. (Although clearly other tests will ?sporadically hit it)

jozkee · 2021-08-11T22:02:06Z

@danmoseley in theory the steps to test this is have a callback in the Timer's queue AFTER you disposed the Timer (which in this case is triggered by PhysicalFilesWatcher.Dispose() -> Timer.Dispose()) AND the file no longer exists. Not 100% sure how to translate that to code/unit test.

danmoseley · 2021-08-12T00:07:55Z

Lots and lots of staging failures. I went through most

failure in Mono Windows Microsoft.Extensions.Hosting.Tests.ConsoleLifetimeExitTests.EnsureEnvironmentExitDoesntHang already tracked by #57184

The test libraries on Catalyst reports show up as "The Helix Work Item failed. Often this is due to a test crash or infrastructure failure. See the Helix Test Logs tab in the Results page of Azure DevOps." but when I look at artefacts, there's a testResults.Xml present with results in it -- @steveisok ? I think that's more interesting to fix than the actual failures.
example failures

hitting the UTF7 issue "Support for UTF-7 is disabled"
"System.PlatformNotSupportedException : System.Drawing.Common is not supported on non-Windows platforms
TestJapaneseCalendarDateParsing
etc

Android has the same issue with not extracting results from the test results XML

these two failures both seem related to appcompat switches?

    <collection total="1" passed="0" failed="1" skipped="0" name="Test collection for System.IO.Tests.Net5CompatSwitchTests" time="0.003">
      <test name="System.IO.Tests.Net5CompatSwitchTests.LegacySwitchIsHonored" type="System.IO.Tests.Net5CompatSwitchTests" method="LegacySwitchIsHonored" time="0.0028879" result="Fail">
        <failure exception-type="Xunit.Sdk.ContainsException">
          <message><![CDATA[Assert.Contains() Failure\nNot found: Net5Compat\nIn value:  System.IO.Strategies.BufferedFileStreamStrategy]]></message>
          <stack-trace><![CDATA[   at System.IO.Tests.Net5CompatSwitchTests.LegacySwitchIsHonored() in /_/src/libraries/System.IO.FileSystem/tests/Net5CompatTests/Net5CompatSwitchTests.cs:line 23
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)]]></stack-trace>
        </failure>

    <collection total="1" passed="0" failed="1" skipped="0" name="Test collection for System.IO.Tests.DisabledFileLockingSwitchTests" time="0.027">
      <test name="System.IO.Tests.DisabledFileLockingSwitchTests.ConfigSwitchIsHonored" type="System.IO.Tests.DisabledFileLockingSwitchTests" method="ConfigSwitchIsHonored" time="0.0268309" result="Fail">
        <failure exception-type="Xunit.Sdk.EqualException">
          <message><![CDATA[Assert.Equal() Failure\nExpected: False\nActual:   True]]></message>
          <stack-trace><![CDATA[   at System.IO.Tests.DisabledFileLockingSwitchTests.ConfigSwitchIsHonored() in /_/src/libraries/System.IO.FileSystem/tests/DisabledFileLockingTests/DisabledFileLockingSwitchTests.cs:line 13
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)]]></stack-trace>

Turkish I

      <test name="System.Tests.CharTests.CasingTest(lowerForm: 'ı', upperForm: 'I', cultureName: \&quot;tr-TR\&quot;)" type="System.Tests.CharTests" method="CasingTest" time="0.0567224" result="Fail">
        <failure exception-type="Xunit.Sdk.EqualException">
          <message><![CDATA[Assert.Equal() Failure\nExpected: 'ı'\nActual:   'i']]></message>
          <stack-trace><![CDATA[   at System.Tests.CharTests.CasingTest(Char lowerForm, Char upperForm, String cultureName) in /_/src/libraries/System.Runtime/tests/System/CharTests.cs:line 1132
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)]]></stack-trace>
        </failure>

various others

No evidence that any of these are caused by this PR. We may be in a bind here where something has broken these specifically because they don't block checkin?

steveisok · 2021-08-12T00:11:49Z

@danmoseley #57114 should address these. There was a prior commit that stopped runtimeconfig.json from being processed. On mobile, we generate a bin file from runtimeconfig.json

danmoseley · 2021-08-12T00:18:10Z

@steveisok thanks, following back to what caused that it seems it was #56486 but it looks like staging passed 100% before that was committed?

danmoseley · 2021-08-12T00:18:44Z

The only non staging failure was the async one fixed in a parallel PR.

Avoid NullRef if ResolveLinkTarget returns null

33af27e

jozkee added the area-Extensions-FileSystem label Aug 11, 2021

jozkee added this to the 6.0.0 milestone Aug 11, 2021

jozkee requested a review from carlossanlop August 11, 2021 20:40

jozkee self-assigned this Aug 11, 2021

carlossanlop approved these changes Aug 11, 2021

View reviewed changes

danmoseley mentioned this pull request Aug 11, 2021

Upgrade SDK to 6.0.100-rc.1.21411.13 #57143

Merged

danmoseley approved these changes Aug 11, 2021

View reviewed changes

ChayoteJarocho mentioned this pull request Aug 11, 2021

when async operation fails with ERROR_OPERATION_ABORTED, we should ignore numBytes that can be != 0 #57229

Merged

danmoseley mentioned this pull request Aug 11, 2021

Microsoft.Extensions.Hosting.Tests.ConsoleLifetimeExitTests failing in Windows mono with PlatformNotSupportedException #57184

Closed

danmoseley merged commit e0f6071 into dotnet:main Aug 12, 2021

jozkee deleted the nullref_physicalfileprovider branch August 12, 2021 00:38

ghost locked as resolved and limited conversation to collaborators Sep 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PhysicalFileProvider: Avoid NullRef if ResolveLinkTarget returns null #57235

PhysicalFileProvider: Avoid NullRef if ResolveLinkTarget returns null #57235
danmoseley merged 1 commit intodotnet:mainfrom
jozkee:nullref_physicalfileprovider

jozkee commented Aug 11, 2021

Uh oh!

ghost commented Aug 11, 2021

Uh oh!

carlossanlop left a comment

Uh oh!

danmoseley left a comment

Uh oh!

jozkee commented Aug 11, 2021 •

edited

Loading

Uh oh!

danmoseley commented Aug 12, 2021 •

edited

Loading

Uh oh!

steveisok commented Aug 12, 2021 •

edited

Loading

Uh oh!

danmoseley commented Aug 12, 2021

Uh oh!

danmoseley commented Aug 12, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

jozkee commented Aug 11, 2021

Uh oh!

ghost commented Aug 11, 2021

Uh oh!

carlossanlop left a comment

Choose a reason for hiding this comment

Uh oh!

danmoseley left a comment

Choose a reason for hiding this comment

Uh oh!

jozkee commented Aug 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danmoseley commented Aug 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steveisok commented Aug 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danmoseley commented Aug 12, 2021

Uh oh!

danmoseley commented Aug 12, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jozkee commented Aug 11, 2021 •

edited

Loading

danmoseley commented Aug 12, 2021 •

edited

Loading

steveisok commented Aug 12, 2021 •

edited

Loading