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

Analysis crash with --no-show-progress option. #598

Open
ismadirolas opened this issue Dec 18, 2024 · 4 comments · Fixed by #599
Open

Analysis crash with --no-show-progress option. #598

ismadirolas opened this issue Dec 18, 2024 · 4 comments · Fixed by #599
Labels
bug Something isn't working

Comments

@ismadirolas
Copy link

ismadirolas commented Dec 18, 2024

Describe the bug
We are executing an analysis over a folder and we get the next error when use the --no-show-progress option:

[ERR] Uncaught exception: AggregateException:One or more errors occurred. (One or more errors occurred. (Value cannot be null. (Parameter 'inputStream'))).
at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions)
   at System.Threading.Tasks.Task.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at System.Threading.Tasks.Task.Wait(TimeSpan timeout, CancellationToken cancellationToken)
   at System.Threading.Tasks.Task.Wait(TimeSpan timeout)
   at Microsoft.ApplicationInspector.Commands.AnalyzeCommand.DoProcessing(IEnumerable`1 fileEntries, CancellationTokenSource cts) in /mnt/vss/_work/1/s/AppInspector/Commands/AnalyzeCommand.cs:line 1132
   at Microsoft.ApplicationInspector.Commands.AnalyzeCommand.GetResult() in /mnt/vss/_work/1/s/AppInspector/Commands/AnalyzeCommand.cs:line 966
   at Microsoft.ApplicationInspector.CLI.Program.RunAnalyzeCommand(CLIAnalyzeCmdOptions cliOptions) in /mnt/vss/_work/1/s/AppInspector.CLI/Program.cs:line 310
   at Microsoft.ApplicationInspector.CLI.Program.VerifyOutputArgsAndRunAnalyze(CLIAnalyzeCmdOptions options) in /mnt/vss/_work/1/s/AppInspector.CLI/Program.cs:line 124
   at Microsoft.ApplicationInspector.CLI.Program.<>c.<Main>b__1_1(CLIAnalyzeCmdOptions cliAnalyzeCmdOptions) in /mnt/vss/_work/1/s/AppInspector.CLI/Program.cs:line 41
   at CommandLine.ParserResultExtensions.MapResult[T1,T2,T3,T4,T5,TResult](ParserResult`1 result, Func`2 parsedFunc1, Func`2 parsedFunc2, Func`2 parsedFunc3, Func`2 parsedFunc4, Func`2 parsedFunc5, Func`2 notParsedFunc)
   at Microsoft.ApplicationInspector.CLI.Program.Main(String[] args) in /mnt/vss/_work/1/s/AppInspector.CLI/Program.cs:line 40
Error: Process completed with exit code 2.

But the thing is that with the same folder and same args without using --no-show-progress option it works correctly.
we don't know what file is causing this (is a big java project) With other projects it doesn't happens.

Thanks in advance.

To Reproduce
Steps to reproduce the behavior:

  1. Run ApplicationInspector with -s . -f json --processing-timeout 900000 --file-timeout 300000 -A -u --no-show-progress -r /rules -i -M 20 -o result.json
  2. See error

Operating Environment (please complete the following information):

  • Application Inspector Version: 1.9.22
  • OS: Linux Ubuntu 20.04
@ismadirolas ismadirolas added the bug Something isn't working label Dec 18, 2024
@gfs
Copy link
Contributor

gfs commented Jan 6, 2025

Thanks for the report. Sorry you're having issues here. At first blush it's not entirely clear why the --no-show-progress option would cause a difference in behavior here - both code paths eventually call into DoProcessing and hit line 1132 with an enumeration of files to process.

I believe that the exception here is actually initially being thrown by a call to the FileEntry constructor in RecursiveExtractor - https://github.com/microsoft/RecursiveExtractor/blob/feaae5560e4c9ba9764505f4a28e0e238895334b/RecursiveExtractor/FileEntry.cs#L63-L66, but because of the task wrapping we're losing a bit of context about where that call is being made and needs to be caught. I believe it's likely to be one of these calls here in EnumerateFileEntries:

if (contents != null)
{
if (_options.DisableCrawlArchives)
{
yield return new FileEntry(srcFile, contents);
}
else
{
// Use MemoryStreamCutoff = 1 to force using FileStream with DeleteOnClose for backing, and avoid memory exhaustion.
ExtractorOptions opts = new()
{
Parallel = false, DenyFilters = _options.FilePathExclusions, MemoryStreamCutoff = 1
};
// This works if the contents contain any kind of file.
// If the file is an archive this gets all the entries it contains.
// If the file is not an archive, the stream is wrapped in a FileEntry container and yielded
foreach (var entry in extractor.Extract(srcFile, contents, opts)) yield return entry;
}
}
, however these calls are already inside a null guard for the stream parameter so I'm not 100% positive that is the culprit.

As for why the behavior may differ - when you use the progress bar ApplicationInspector pre-enumerates a collection of all the files to scan using an indeterminate progress bar (since we don't yet know how many files there are), then once the set of files to scan are known uses a determinate progress bar to scan them. When running without the progress bar, we can save some time and memory by creating a lazy enumeration and then scanning the files as they are enumerated.

May simply require that I attempt to replicate the behavior by scanning a large set of files.

gfs added a commit that referenced this issue Jan 8, 2025
Was able to reproduce overflow exceptions thrown here causing end to execution, I think this is potentially the same root cause of issue #598's report of null streams causing a crash as well.
gfs added a commit that referenced this issue Jan 10, 2025
* Add Catch around Recursive Extractor calls

Was able to reproduce overflow exceptions thrown here causing end to execution, I think this is potentially the same root cause of issue #598's report of null streams causing a crash as well.

* Fix Capitalization

* Clamp GetLocation to end of file
@gfs gfs closed this as completed in #599 Jan 10, 2025
@ismadirolas
Copy link
Author

Hello @gfs ,

When do you plan to release this fix for testing with our project?

Thank you.

@gfs gfs reopened this Jan 20, 2025
@gfs
Copy link
Contributor

gfs commented Jan 20, 2025

Hey @ismadirolas,

Sorry for the delay - I believe that the fix in #599 will resolve your issue, but we ran into some unexpected issues with the release process. I'm aiming to have those issues resolved so we can get the new version out this week.

Thanks.

@gfs
Copy link
Contributor

gfs commented Jan 23, 2025

@ismadirolas We released a new version yesterday with the change from #599. Please let me know if that does not resolve the issue you were hitting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants