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

BUGFIX : Resolve hangs due to unhandled exceptions during multithreaded analysis file hashing phase. #2600

Merged
merged 3 commits into from
Dec 29, 2022

Conversation

shaopeng-gh
Copy link
Collaborator

When exception happens, below 2 lines will not run:

image

I can reproduce this when debugging below line will be waiting, not ending at all:

image

@@ -882,6 +882,45 @@ public void MultithreadedAnalyzeCommandBase_TargetFileSizeTestCases()
}
}

[Fact]
public void MultithreadedAnalyzeCommandBase_ErrorWhenHashing()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MultithreadedAnalyzeCommandBase_ErrorWhenHashing

this is to add a test for previous pr about NullReferenceException.
without that fix, this test will fail with NullReferenceException

finally
{
readyToScanChannel.Writer.Complete();
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

used the same code for enumeration phase.

Comment on lines +559 to +563
catch (Exception e)
{
Errors.LogUnhandledEngineException(_rootContext, e);
ThrowExitApplicationException(_rootContext, ExitReason.UnhandledExceptionInEngine);
}

Check notice

Code scanning / CodeQL

Generic catch clause

Generic catch clause.
Copy link
Member

@michaelcfanning michaelcfanning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes meant to note that we need to review all channel writes and reads to ensure that completion occurs in any code path with unhandled exception.

@michaelcfanning michaelcfanning merged commit 08adc24 into main Dec 29, 2022
@michaelcfanning michaelcfanning deleted the users/shaopeng-gh/fixhang branch December 29, 2022 16:22
This was referenced Feb 21, 2023
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.

None yet

2 participants