Skip to content

Unlock before returning#526

Merged
lukasIO merged 1 commit intomainfrom
lukas/fix-lock-return
Dec 6, 2022
Merged

Unlock before returning#526
lukasIO merged 1 commit intomainfrom
lukas/fix-lock-return

Conversation

@lukasIO
Copy link
Copy Markdown
Contributor

@lukasIO lukasIO commented Dec 6, 2022

No description provided.

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Dec 6, 2022

⚠️ No Changeset found

Latest commit: 51fc216

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@lukasIO lukasIO requested a review from davidzhao December 6, 2022 07:50
@lukasIO lukasIO merged commit fcc4b12 into main Dec 6, 2022
@lukasIO lukasIO deleted the lukas/fix-lock-return branch December 6, 2022 09:10
dorr91
dorr91 reviewed Dec 6, 2022
View reviewed changes
src/room/Room.ts
try {
if (this.state === ConnectionState.Disconnected) {
log.debug('already disconnected');
unlock();
Copy link
Copy Markdown

@dorr91 dorr91 Dec 6, 2022

Choose a reason for hiding this comment

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

Hey @lukasIO I think the unlock in the finally clause should catch this early return. Does unlocking twice cause issues?

Copy link
Copy Markdown
Contributor Author

@lukasIO lukasIO Dec 6, 2022

Choose a reason for hiding this comment

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

ah, of course, you're right. was overly eager to catch those early returns...
shouldn't cause any problems, but will revert in order to keep it clean.

lukasIO added a commit that referenced this pull request Dec 6, 2022
@lukasIO
Revert "unlock before returning (#526)"
e5e2a7d 
This reverts commit fcc4b12.
lukasIO added a commit that referenced this pull request Dec 6, 2022
@lukasIO
Revert "unlock before returning (#526)" (#527)
ed3b30d 
This reverts commit fcc4b12.
afgarcia86 pushed a commit to afgarcia86/client-sdk-js that referenced this pull request Mar 19, 2026
@Ocupe
underline links in chat message (livekit#526)
598442f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davidzhao davidzhao davidzhao approved these changes

+1 more reviewer

@dorr91 dorr91 dorr91 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@lukasIO @davidzhao @dorr91