Skip to content
This repository has been archived by the owner on Apr 20, 2023. It is now read-only.

Create locking mechanism to allow for sequential execs #45

Open
wants to merge 1 commit into
base: earthly-main
Choose a base branch
from

Conversation

alexcb
Copy link

@alexcb alexcb commented Jun 16, 2021

Create two magic-string args for specifying lock and unlock ops.
Locks are referenced with a string name, additional locks on a
previously locked lock of the same name are ignored.

Signed-off-by: Alex Couture-Beil [email protected]

@alexcb alexcb marked this pull request as ready for review June 16, 2021 23:02
@alexcb alexcb requested a review from vladaionescu June 16, 2021 23:02
Create two magic-string args for specifying lock and unlock ops.
Locks are referenced with a string name, additional locks on a
previously locked lock of the same name are ignored.

Signed-off-by: Alex Couture-Beil <[email protected]>
Copy link
Member

@vladaionescu vladaionescu left a comment

Choose a reason for hiding this comment

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

Hope that that Any context cancels when expected. Might be worth testing that out manually (kill a client in the middle of the build to see if LOCALLY hangs forever).

select {
case <-l.ctx.Done():
// unlock locks from closed connections
l.current = ""
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if this is absolutely needed - but feel like there might be an edge case that we could be missing?

Suggested change
l.current = ""
l.current = ""
l.ctx = nil

@vladaionescu
Copy link
Member

Is this still ongoing?

@alexcb
Copy link
Author

alexcb commented Jul 28, 2021

Is this still ongoing?

It's on the backburner, but still needs the context-aware lock to unlock if the session is lost/prematurely disconnected.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants