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

fix unlock in addDevice of membership #3353

Merged
merged 1 commit into from
Nov 27, 2021
Merged

Conversation

jidalong
Copy link
Member

@jidalong jidalong commented Nov 24, 2021

What type of PR is this?
/kind bug

What this PR does / why we need it:
a lock was in line 195-197:

if delta {
  context.Lock(device.ID)
}

unlock was in line 251-253:

if delta {
  context.Unlock(device.ID)
}

PR #3348 add log err if convert membership failed , if we use "continue" in line 244 before unlock, it may cause not relase lock as mentioned, so here we should remove it.

Does this PR introduce a user-facing change?:
NONE


@kubeedge-bot kubeedge-bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Nov 24, 2021
@jidalong jidalong force-pushed the memship branch 4 times, most recently from 4c0d0ae to 04de5db Compare November 24, 2021 02:12
Signed-off-by: jidalong <jidalong@cmss.chinamobile.com>
Signed-off-by: jidalong <jidalong_yewu@cmss.chinamobile.com>
@gy95
Copy link
Member

gy95 commented Nov 25, 2021

LGTM

@jidalong jidalong changed the title fix unlock in addDevice of membership 修复 addDevice锁未释放bug(fix unlock in addDevice of membership) Nov 25, 2021
@jidalong
Copy link
Member Author

LGTM

@gy95 Sorry to disturb, maybe kubeedge project need label '/lgtm' instead of 'LGTM' :)

@gy95
Copy link
Member

gy95 commented Nov 25, 2021

this is waiting for approve @fisherxu :)

@jidalong
Copy link
Member Author

this is waiting for approve @fisherxu :)

OKey~Thanks

@zc2638
Copy link
Member

zc2638 commented Nov 26, 2021

Hello @jidalong,
Because this project is internationally oriented, and English is the international common language.
It is recommended that all submissions contain only English.
Could you change the title to pure English?
Thanks.

@jidalong jidalong changed the title 修复 addDevice锁未释放bug(fix unlock in addDevice of membership) fix unlock in addDevice of membership Nov 26, 2021
@jidalong
Copy link
Member Author

Hello @jidalong, Because this project is internationally oriented, and English is the international common language. It is recommended that all submissions contain only English. Could you change the title to pure English? Thanks.

@zc2638 Okey,Done.

@zc2638
Copy link
Member

zc2638 commented Nov 26, 2021

/lgtm
/assign @fisherxu

@kubeedge-bot kubeedge-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 26, 2021
@gy95
Copy link
Member

gy95 commented Nov 27, 2021

/lgtm

Copy link
Member

@fisherxu fisherxu left a comment

Choose a reason for hiding this comment

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

/approve

@kubeedge-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fisherxu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubeedge-bot kubeedge-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 27, 2021
@kubeedge-bot kubeedge-bot merged commit b892af2 into kubeedge:master Nov 27, 2021
@jidalong jidalong deleted the memship branch November 29, 2021 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants