Skip to content

Fix the race on the registered conversion paths in conversion.Convert#545

Merged
ulucinar merged 1 commit intocrossplane:mainfrom
ulucinar:fix-conversion-race
Oct 28, 2025
Merged

Fix the race on the registered conversion paths in conversion.Convert#545
ulucinar merged 1 commit intocrossplane:mainfrom
ulucinar:fix-conversion-race

Conversation

@ulucinar
Copy link
Copy Markdown
Collaborator

@ulucinar ulucinar commented Oct 24, 2025

Description of your changes

Multiple Go routines can race on the registered conversion paths when in-place sorts are performed in conversion.Convert. Although we could not reproduce these issues like the one reported in crossplane-contrib/provider-upjet-aws#1714, observations done by @jonathan-innis here using a provider package with the fix proposed in this PR suggest that this data race could be the underlying reason behind similar failures.

This PR fixes the data race by making a copy of the list of the registered conversion paths before they are sorted in-place.

I have:

  • Read and followed Upjet's contribution process.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

We have tested the data race fix using the provider package xpkg.upbound.io/upbound/provider-aws-lambda:v1.20.5-1.gf4470285b. There are logging differences between that implementation and the one proposed here. The test image has more verbose logging, which is not suggested with this PR due to concerns with PII.

Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
Copy link
Copy Markdown
Member

@sergenyalcin sergenyalcin left a comment

Choose a reason for hiding this comment

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

Thank you very much @ulucinar for catching and fixing this. LGTM!

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.

2 participants