fix data-races when running analysis #477

matheusalcantarazup · 2021-06-08T13:42:29Z

- What I did
Previously when we start an analysis of language/tool we controlled the
state of execution using the monitor package, but many objects use the
same instance of monitor doing updates and reads concurrent resulting
in possible data races. This commit drops the monitor package and replace
to use the sync.WaitGroup to control the state of go routines.
An improvement was also made to control the timeout of analysis using
time.After function to receive the channel when timeout occurred or close
the done channel when analysis finish. An mutex was added on Service
to avoid data races when adding errors on Analysis.

PS: This PR has many file changes, but is because I drop the monitor package and I needed to remove the reference in many files.

- How to verify it

- Description for the changelog

Previously when we start an analysis of language/tool we controlled the state of execution using the monitor package, but many objects use the same instance of monitor doing updates and reads concurrent resulting in possible data races. This commit drops the monitor package and replace to use the `sync.WaitGroup` to control the state of go routines. An improvement was also made to control the timeout of analysis using `time.After` function to receive the channel when timeout occurred or close the `done` channel when analysis finish. An mutex was added on Service to avoid data races when adding errors on Analysis.

* fix data-races when running analysis (#477) * fix possible wrong path concat on windows * enable data race detection on make test * fix data-races when running analysis Previously when we start an analysis of language/tool we controlled the state of execution using the monitor package, but many objects use the same instance of monitor doing updates and reads concurrent resulting in possible data races. This commit drops the monitor package and replace to use the `sync.WaitGroup` to control the state of go routines. An improvement was also made to control the timeout of analysis using `time.After` function to receive the channel when timeout occurred or close the `done` channel when analysis finish. An mutex was added on Service to avoid data races when adding errors on Analysis. * improvement on Swift rules description (#479) * feature/dependency-check (#478) * Adding owasp dependency check formatter * Adding tests and fixing lint * Adding flag to enable owasp dependency check * Fixing pipeline errors * Fixing some errors * Updating devkit version * Feature/dotnet cli (#480) * Adding dotnet cli dependency check * Fixing lint errors * Adding lisence header * Improving security code scan * Adding validation to not found solution in scs, adding license headers * Adding code in security code scan * Updating csharp example with vulnerable dependencies, adding validation to failed build in security code scan * Fixing some errors * Adding code, line and filepath in dotnet cli. Fixing some errors * Updating horusec json * Fixing commit authors issues * Fixing some issues found during tests * Adding validation to dotnetcli output * Fixing lint error * Fixing lint errors * Fixing lint error * Updating horusec config json * Updating go modules and adding missing unity test * Fixing error to remove .horusec * [skip ci] Update versioning file Co-authored-by: matheusalcantarazup <84723211+matheusalcantarazup@users.noreply.github.com> Co-authored-by: nathanmartinszup <63246935+nathanmartinszup@users.noreply.github.com>

* fix data-races when running analysis (#477) * fix possible wrong path concat on windows * enable data race detection on make test * fix data-races when running analysis Previously when we start an analysis of language/tool we controlled the state of execution using the monitor package, but many objects use the same instance of monitor doing updates and reads concurrent resulting in possible data races. This commit drops the monitor package and replace to use the `sync.WaitGroup` to control the state of go routines. An improvement was also made to control the timeout of analysis using `time.After` function to receive the channel when timeout occurred or close the `done` channel when analysis finish. An mutex was added on Service to avoid data races when adding errors on Analysis. * improvement on Swift rules description (#479) * feature/dependency-check (#478) * Adding owasp dependency check formatter * Adding tests and fixing lint * Adding flag to enable owasp dependency check * Fixing pipeline errors * Fixing some errors * Updating devkit version * Feature/dotnet cli (#480) * Adding dotnet cli dependency check * Fixing lint errors * Adding lisence header * Improving security code scan * Adding validation to not found solution in scs, adding license headers * Adding code in security code scan * Updating csharp example with vulnerable dependencies, adding validation to failed build in security code scan * Fixing some errors * Adding code, line and filepath in dotnet cli. Fixing some errors * Updating horusec json * Fixing commit authors issues * Fixing some issues found during tests * Adding validation to dotnetcli output * Fixing lint error * Fixing lint errors * Fixing lint error * Updating horusec config json * Updating go modules and adding missing unity test * Fixing error to remove .horusec * [skip ci] Update versioning file * avoid time.Sleep to log analysis timeout status (#482) Previously, to warn the user that the analysis is still running, we used time.Sleep to print how much time is left for the timeout, however, if the analysis has already been completed, we still need to wait for the end of time.Sleep to finish the analysis. This change removes time.Sleep and uses time.Tick to print the message, so, if the analysis is finished before the next retry, we do not lock the analysis with time.Sleep. * Feature/nancy (#483) * Adding nancy dependency check for go * Adding nancy unity tests * Adding vulnerable dependencies in go example project * Fixing errors found during tests * Fixing unity tests and pipeline errors * Updating devkit version * Fixing pipeline errors * Updating go dockerfile to use nancy binary from github * Fixing go sum * Updating config json Co-authored-by: matheusalcantarazup <84723211+matheusalcantarazup@users.noreply.github.com> Co-authored-by: nathanmartinszup <63246935+nathanmartinszup@users.noreply.github.com> Co-authored-by: wilian <wilian.silva@zup.com.br> Co-authored-by: nathanmartinszup <nathan.martins@zup.com.br>

* fix data-races when running analysis (#477) * fix possible wrong path concat on windows * enable data race detection on make test * fix data-races when running analysis Previously when we start an analysis of language/tool we controlled the state of execution using the monitor package, but many objects use the same instance of monitor doing updates and reads concurrent resulting in possible data races. This commit drops the monitor package and replace to use the `sync.WaitGroup` to control the state of go routines. An improvement was also made to control the timeout of analysis using `time.After` function to receive the channel when timeout occurred or close the `done` channel when analysis finish. An mutex was added on Service to avoid data races when adding errors on Analysis. * improvement on Swift rules description (#479) * feature/dependency-check (#478) * Adding owasp dependency check formatter * Adding tests and fixing lint * Adding flag to enable owasp dependency check * Fixing pipeline errors * Fixing some errors * Updating devkit version * Feature/dotnet cli (#480) * Adding dotnet cli dependency check * Fixing lint errors * Adding lisence header * Improving security code scan * Adding validation to not found solution in scs, adding license headers * Adding code in security code scan * Updating csharp example with vulnerable dependencies, adding validation to failed build in security code scan * Fixing some errors * Adding code, line and filepath in dotnet cli. Fixing some errors * Updating horusec json * Fixing commit authors issues * Fixing some issues found during tests * Adding validation to dotnetcli output * Fixing lint error * Fixing lint errors * Fixing lint error * Updating horusec config json * Updating go modules and adding missing unity test * Fixing error to remove .horusec * [skip ci] Update versioning file * avoid time.Sleep to log analysis timeout status (#482) Previously, to warn the user that the analysis is still running, we used time.Sleep to print how much time is left for the timeout, however, if the analysis has already been completed, we still need to wait for the end of time.Sleep to finish the analysis. This change removes time.Sleep and uses time.Tick to print the message, so, if the analysis is finished before the next retry, we do not lock the analysis with time.Sleep. * Feature/nancy (#483) * Adding nancy dependency check for go * Adding nancy unity tests * Adding vulnerable dependencies in go example project * Fixing errors found during tests * Fixing unity tests and pipeline errors * Updating devkit version * Fixing pipeline errors * Updating go dockerfile to use nancy binary from github * Fixing go sum * Updating config json * Adding option to txt output * Adding option to txt output Signed-off-by: nathanmartinszup <nathan.martins@zup.com.br> Co-authored-by: matheusalcantarazup <84723211+matheusalcantarazup@users.noreply.github.com> Co-authored-by: wilian <wilian.silva@zup.com.br> Co-authored-by: Nathan Tavares Nascimento <nathan.nascimento@zup.com.br>

* fix data-races when running analysis (#477) * fix possible wrong path concat on windows * enable data race detection on make test * fix data-races when running analysis Previously when we start an analysis of language/tool we controlled the state of execution using the monitor package, but many objects use the same instance of monitor doing updates and reads concurrent resulting in possible data races. This commit drops the monitor package and replace to use the `sync.WaitGroup` to control the state of go routines. An improvement was also made to control the timeout of analysis using `time.After` function to receive the channel when timeout occurred or close the `done` channel when analysis finish. An mutex was added on Service to avoid data races when adding errors on Analysis. * improvement on Swift rules description (#479) * feature/dependency-check (#478) * Adding owasp dependency check formatter * Adding tests and fixing lint * Adding flag to enable owasp dependency check * Fixing pipeline errors * Fixing some errors * Updating devkit version * Feature/dotnet cli (#480) * Adding dotnet cli dependency check * Fixing lint errors * Adding lisence header * Improving security code scan * Adding validation to not found solution in scs, adding license headers * Adding code in security code scan * Updating csharp example with vulnerable dependencies, adding validation to failed build in security code scan * Fixing some errors * Adding code, line and filepath in dotnet cli. Fixing some errors * Updating horusec json * Fixing commit authors issues * Fixing some issues found during tests * Adding validation to dotnetcli output * Fixing lint error * Fixing lint errors * Fixing lint error * Updating horusec config json * Updating go modules and adding missing unity test * Fixing error to remove .horusec * [skip ci] Update versioning file * avoid time.Sleep to log analysis timeout status (#482) Previously, to warn the user that the analysis is still running, we used time.Sleep to print how much time is left for the timeout, however, if the analysis has already been completed, we still need to wait for the end of time.Sleep to finish the analysis. This change removes time.Sleep and uses time.Tick to print the message, so, if the analysis is finished before the next retry, we do not lock the analysis with time.Sleep. * Feature/nancy (#483) * Adding nancy dependency check for go * Adding nancy unity tests * Adding vulnerable dependencies in go example project * Fixing errors found during tests * Fixing unity tests and pipeline errors * Updating devkit version * Fixing pipeline errors * Updating go dockerfile to use nancy binary from github * Add set log file to global command Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * Add set log file to global command Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * remove ctrl v error Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * refactor cyclomatic function Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * Fixing lint errors Signed-off-by: nathanmartinszup <nathan.martins@zup.com.br> * fix tests Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * Add LogOutput Tests Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * Fix tests Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * altering fileName creation to use filepath.Join Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * removing comments from tests Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * Change logpath flage name to log-file-path and refactor on logSetOutput * Adding more tests and system call interface Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * Save work Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * fix viper config Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * Add trivy as new tool to generic analyzers Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * Att gomod and add setSystemCall * Add license to trivy files * Fix tools to ignore from merge commit error Co-authored-by: matheusalcantarazup <84723211+matheusalcantarazup@users.noreply.github.com> Co-authored-by: nathanmartinszup <63246935+nathanmartinszup@users.noreply.github.com> Co-authored-by: wilian <wilian.silva@zup.com.br> Co-authored-by: nathanmartinszup <nathan.martins@zup.com.br>

* fix data-races when running analysis (#477) * fix possible wrong path concat on windows * enable data race detection on make test * fix data-races when running analysis Previously when we start an analysis of language/tool we controlled the state of execution using the monitor package, but many objects use the same instance of monitor doing updates and reads concurrent resulting in possible data races. This commit drops the monitor package and replace to use the `sync.WaitGroup` to control the state of go routines. An improvement was also made to control the timeout of analysis using `time.After` function to receive the channel when timeout occurred or close the `done` channel when analysis finish. An mutex was added on Service to avoid data races when adding errors on Analysis. * improvement on Swift rules description (#479) * feature/dependency-check (#478) * Adding owasp dependency check formatter * Adding tests and fixing lint * Adding flag to enable owasp dependency check * Fixing pipeline errors * Fixing some errors * Updating devkit version * Feature/dotnet cli (#480) * Adding dotnet cli dependency check * Fixing lint errors * Adding lisence header * Improving security code scan * Adding validation to not found solution in scs, adding license headers * Adding code in security code scan * Updating csharp example with vulnerable dependencies, adding validation to failed build in security code scan * Fixing some errors * Adding code, line and filepath in dotnet cli. Fixing some errors * Updating horusec json * Fixing commit authors issues * Fixing some issues found during tests * Adding validation to dotnetcli output * Fixing lint error * Fixing lint errors * Fixing lint error * Updating horusec config json * Updating go modules and adding missing unity test * Fixing error to remove .horusec * [skip ci] Update versioning file * avoid time.Sleep to log analysis timeout status (#482) Previously, to warn the user that the analysis is still running, we used time.Sleep to print how much time is left for the timeout, however, if the analysis has already been completed, we still need to wait for the end of time.Sleep to finish the analysis. This change removes time.Sleep and uses time.Tick to print the message, so, if the analysis is finished before the next retry, we do not lock the analysis with time.Sleep. * Feature/nancy (#483) * Adding nancy dependency check for go * Adding nancy unity tests * Adding vulnerable dependencies in go example project * Fixing errors found during tests * Fixing unity tests and pipeline errors * Updating devkit version * Fixing pipeline errors * Updating go dockerfile to use nancy binary from github * Fixing go sum * Updating config json * Adding option to txt output * Adding option to txt output Signed-off-by: nathanmartinszup <nathan.martins@zup.com.br> Co-authored-by: matheusalcantarazup <84723211+matheusalcantarazup@users.noreply.github.com> Co-authored-by: wilian <wilian.silva@zup.com.br> Co-authored-by: Nathan Tavares Nascimento <nathan.nascimento@zup.com.br>

* fix data-races when running analysis (#477) * fix possible wrong path concat on windows * enable data race detection on make test * fix data-races when running analysis Previously when we start an analysis of language/tool we controlled the state of execution using the monitor package, but many objects use the same instance of monitor doing updates and reads concurrent resulting in possible data races. This commit drops the monitor package and replace to use the `sync.WaitGroup` to control the state of go routines. An improvement was also made to control the timeout of analysis using `time.After` function to receive the channel when timeout occurred or close the `done` channel when analysis finish. An mutex was added on Service to avoid data races when adding errors on Analysis. * improvement on Swift rules description (#479) * feature/dependency-check (#478) * Adding owasp dependency check formatter * Adding tests and fixing lint * Adding flag to enable owasp dependency check * Fixing pipeline errors * Fixing some errors * Updating devkit version * Feature/dotnet cli (#480) * Adding dotnet cli dependency check * Fixing lint errors * Adding lisence header * Improving security code scan * Adding validation to not found solution in scs, adding license headers * Adding code in security code scan * Updating csharp example with vulnerable dependencies, adding validation to failed build in security code scan * Fixing some errors * Adding code, line and filepath in dotnet cli. Fixing some errors * Updating horusec json * Fixing commit authors issues * Fixing some issues found during tests * Adding validation to dotnetcli output * Fixing lint error * Fixing lint errors * Fixing lint error * Updating horusec config json * Updating go modules and adding missing unity test * Fixing error to remove .horusec * [skip ci] Update versioning file * avoid time.Sleep to log analysis timeout status (#482) Previously, to warn the user that the analysis is still running, we used time.Sleep to print how much time is left for the timeout, however, if the analysis has already been completed, we still need to wait for the end of time.Sleep to finish the analysis. This change removes time.Sleep and uses time.Tick to print the message, so, if the analysis is finished before the next retry, we do not lock the analysis with time.Sleep. * Feature/nancy (#483) * Adding nancy dependency check for go * Adding nancy unity tests * Adding vulnerable dependencies in go example project * Fixing errors found during tests * Fixing unity tests and pipeline errors * Updating devkit version * Fixing pipeline errors * Updating go dockerfile to use nancy binary from github * Add set log file to global command Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * Add set log file to global command Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * remove ctrl v error Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * refactor cyclomatic function Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * Fixing lint errors Signed-off-by: nathanmartinszup <nathan.martins@zup.com.br> * fix tests Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * Add LogOutput Tests Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * Fix tests Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * altering fileName creation to use filepath.Join Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * removing comments from tests Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * Change logpath flage name to log-file-path and refactor on logSetOutput * Adding more tests and system call interface Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * Save work Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * fix viper config Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * Add trivy as new tool to generic analyzers Signed-off-by: Ian Cardoso <ian.cardoso@zup.com.br> * Att gomod and add setSystemCall * Add license to trivy files * Fix tools to ignore from merge commit error Co-authored-by: matheusalcantarazup <84723211+matheusalcantarazup@users.noreply.github.com> Co-authored-by: nathanmartinszup <63246935+nathanmartinszup@users.noreply.github.com> Co-authored-by: wilian <wilian.silva@zup.com.br> Co-authored-by: nathanmartinszup <nathan.martins@zup.com.br>

Previously we control all execution of tools using a `monitor` package, but this package contained a lot of data races that was fixed on #477 and the monitor retry count flag was no longer necessary, since this was used to control the timeout counter. This commit mark monitor-retry-count flag as hidden and deprecated, so new users will not see this flag and users that is currently using this flag on pipelines will a warning saying to use just --analysis-timeout flag. Signed-off-by: Matheus Alcantara <matheus.alcantara@zup.com.br>

Previously we control all execution of tools using a `monitor` package, but this package contained a lot of data races that was fixed on #477 and the monitor retry count flag was no longer necessary, since this was used to control the timeout counter. This commit mark monitor-retry-count flag as hidden and deprecated, so new users will not see this flag and users that is currently using this flag on pipelines will see a warning saying to use just --analysis-timeout flag. Signed-off-by: Matheus Alcantara <matheus.alcantara@zup.com.br>

Previously we control all execution of tools using a monitor package, but this package contained a lot of data races that was fixed on #477 and the monitor retry count flag was no longer necessary, since this was used to control the timeout counter. This commit mark monitor-retry-count flag as hidden and deprecated, so new users will not see this flag and users that is currently using this flag on pipelines will see a warning saying to use just --analysis-timeout flag. Signed-off-by: Matheus Alcantara <matheus.alcantara@zup.com.br>

matheusalcantarazup requested review from lucasbrunozup, lucasgarciazup, nathanmartinszup, nathannascimentozup, tiagoangelozup and wiliansilvazup as code owners June 8, 2021 13:42

matheusalcantarazup added 2 commits June 8, 2021 11:21

fix possible wrong path concat on windows

2154336

enable data race detection on make test

e79df68

matheusalcantarazup force-pushed the fix-data-races branch from cec3432 to de68b5f Compare June 8, 2021 14:23

nathanmartinszup approved these changes Jun 8, 2021

View reviewed changes

matheusalcantarazup force-pushed the fix-data-races branch from de68b5f to c97451a Compare June 8, 2021 19:45

matheusalcantarazup force-pushed the fix-data-races branch from c97451a to b51aca3 Compare June 8, 2021 20:03

nathanmartinszup merged commit 05242f7 into develop Jun 9, 2021

nathanmartinszup deleted the fix-data-races branch June 9, 2021 11:20

wiliansilvazup mentioned this pull request Jun 21, 2021

Develop #481

Merged

matheusalcantarazup mentioned this pull request Dec 3, 2021

cli:chore - deprecate monitor-retry-count flag #841

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix data-races when running analysis #477

fix data-races when running analysis #477

matheusalcantarazup commented Jun 8, 2021 •

edited

Loading

fix data-races when running analysis #477

fix data-races when running analysis #477

Conversation

matheusalcantarazup commented Jun 8, 2021 • edited Loading

matheusalcantarazup commented Jun 8, 2021 •

edited

Loading