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

Action lint fails occasionally - fatal error: concurrent map writes #370

Closed
zilinjak opened this issue Oct 26, 2023 · 6 comments
Closed

Comments

@zilinjak
Copy link

zilinjak commented Oct 26, 2023

Hello,
when using rhysd/actionlint:latest docker image to analyze workflows- https://hub.docker.com/r/rhysd/actionlint

The action sometimes fails out of nowhere without any reason. The log can be seen here

Thanks for any advices

@ReinAchten-TomTom
Copy link

ReinAchten-TomTom commented Oct 31, 2023

We notice the same issue and for whatever reason we've started to see it more often recently. Can reproduce this quite easily locally when running actionlint in context of a repo with several (14 in my case) GitHub workflows:

while true; do
	output=`actionlint -format '{{json .}}' 2>&1`
	if [[ $output == *"fatal error: concurrent map"* ]]; then
		echo "FATAL ERROR SEEN"
		break
	fi
done

It's a bug (accessing a non thread-safe container concurrently) in actionlint that merely happens when a custom error message format is provided through -format. The map being accessed concurrently is ErrorFormatter.rules (see here).

We've noticed at least the following race condition regarding this map when RegisterRule is called from multiple concurrent check functions calls.

I'm not sufficiently familiar with the codebase to propose a good solution. A naive solution would be to use a thread-safe map instead, but I can't properly judge the performance implications.

@ReinAchten-TomTom
Copy link

@rhysd is this something you could look into or suggest how to best fix? I'm not proficient in golang, but can give it a try if you provide a hint. Thnx! 🙏

@rhysd
Copy link
Owner

rhysd commented Nov 7, 2023

Let me check this issue this weekend. I don't have enough time to handle all projects for now.

@noahnu
Copy link

noahnu commented Dec 20, 2023

I've run into this as well. Using an older version of actionlint in the meantime. Looking into the issue, it seems switching to sync.Map for the rules map may help?

@rhysd rhysd closed this as completed in d80a2a3 Feb 16, 2024
@rhysd
Copy link
Owner

rhysd commented Feb 16, 2024

I apologize for the delay. I could fix this at d80a2a3. @ReinAchten-TomTom's comment was so helpful to fix this.

@maikelvdh
Copy link

maikelvdh commented Feb 19, 2024

@rhysd thanks a lot for addressing this problem! is there any plan to release a new tagged version soon? 🙏

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

No branches or pull requests

5 participants