Skip to content

fix: add lock to project-related operations to avoid race condition issues - #8985

Merged
ZxBing0066 merged 5 commits into
developfrom
fix/add-lock-to-project-operations
Aug 14, 2025
Merged

ZxBing0066 merged 5 commits into
developfrom
fix/add-lock-to-project-operations

Conversation

@ZxBing0066

@ZxBing0066 ZxBing0066 commented Aug 13, 2025 •

Copy link
Copy Markdown
Member

INS-1107

Background

  • There are many operations to write the project data
    • Create project
    • Delete project
    • Update project
    • Sync projects(will get the remote projects then change remote projects to local when they don't exist in the remote, and create projects when they don't exist in the local db)
  • These operations will cause race condition issues
    • Duplicate remote projects
      • create remote project -> create corresponding project in the db
      • SSE event triggered before the API response, it will create the project in the db
      • Now we have two duplicate remote projects
    • Wrong project type
      • create remote project A -> create corresponding project A in the db
      • create remote project B -> create corresponding project B in the db
      • SSE event for project A triggered before project B was created, and check after project B was created in the db
      • The first created project B will be converted to a local project, and a new remote project B will be created

Sync projects are not only called by SSE, but also when we switch the organization or via an action.

Changes

  • Add a lock for all the project-related operations
  • Fix the issue that, when the project name is changed but there are projects with the same name in the remote, sync projects don't update its name correctly

@ZxBing0066 ZxBing0066 self-assigned this Aug 13, 2025
return wrappedFn as T;
};

return { wrapWithLock, lock, unlock };

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

After we unify all the project related APIs into the same entry, lock and unlock could be removed.

@cwangsmv

cwangsmv commented Aug 13, 2025 •

Copy link
Copy Markdown
Contributor

The mutex lock implementation have potential dead lock situation

function A = async() =>  {
  await lock();
  try {
    await B(); // this will cause dead lock
  } finally {
    await unlock();
  }
}

function B = async() =>  {
  await lock(); // wait forever
  try {
    // do anything here
  } finally {
    await unlock();
  }
}

A();

We shall make sure no lock function will call another function that acquires the same lock.

@ZxBing0066

Copy link
Copy Markdown
Member Author

The mutex lock implementation have potential dead lock situation

function A = async() =>  {
  await lock();
  try {
    await B(); // this will cause dead lock
  } finally {
    await unlock();
  }
}

function B = async() =>  {
  await lock(); // wait forever
  try {
    // do anything here
  } finally {
    await unlock();
  }
}

A();

We shall make sure no lock function will call another function that acquires the same lock.

@cwangsmv yes, but I think that's a bug of the implementation rather than a bug of the lock. Just like we return a new promise but forget to resolve it.

CurryYangxx
CurryYangxx previously approved these changes Aug 14, 2025
yaoweiprc
yaoweiprc previously approved these changes Aug 14, 2025
@ZxBing0066
ZxBing0066 enabled auto-merge (squash) August 14, 2025 09:20
cwangsmv
cwangsmv previously approved these changes Aug 14, 2025
@ZxBing0066
ZxBing0066 disabled auto-merge August 14, 2025 09:34
@ZxBing0066
ZxBing0066 dismissed stale reviews from cwangsmv, yaoweiprc, and CurryYangxx via d636895 August 14, 2025 09:39
@ZxBing0066
ZxBing0066 enabled auto-merge (squash) August 14, 2025 09:51
@ZxBing0066
ZxBing0066 merged commit c61eb98 into develop Aug 14, 2025
@ZxBing0066
ZxBing0066 deleted the fix/add-lock-to-project-operations branch August 14, 2025 10:07
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.

4 participants