From cb718d486fec0b0d18860c087fa6e8eeaf9803e0 Mon Sep 17 00:00:00 2001 From: Noah Metz Date: Fri, 23 Jun 2023 21:57:26 -0600 Subject: [PATCH] fixed bug in lockable and moved delegation_map to lockable --- lockable.go | 60 ++++++++++++++++++++++++++++++++++++----------- thread.go | 63 +++++++++++++++----------------------------------- thread_test.go | 13 ++++++++++- 3 files changed, 77 insertions(+), 59 deletions(-) diff --git a/lockable.go b/lockable.go index 0ec2a3d..2584a55 100644 --- a/lockable.go +++ b/lockable.go @@ -5,11 +5,16 @@ import ( "encoding/json" ) -// Any struct that wants to hold a lock must implement this interface +// LockHolderState is the interface that any node that wants to posses locks must implement +// +// ReturnLock returns the node that held the resource pointed to by ID before this node, +// or nil if the resource was unlocked previously +// +// AllowedToTakeLock returns true if the node pointed to by ID is allowed to take a lock from this node type LockHolderState interface { - OriginalLockHolder(id NodeID) GraphNode - AllowedToTakeLock(id NodeID) bool - RecordLockHolder(id NodeID, lock_holder GraphNode) + ReturnLock(resource_id NodeID) GraphNode + AllowedToTakeLock(node_id NodeID, resource_id NodeID) bool + RecordLockHolder(resource_id NodeID, lock_holder GraphNode) } // Any node that wants to be connected to the lockable DAG must implement this interface @@ -29,6 +34,7 @@ type BaseLockableState struct { owner GraphNode requirements []Lockable dependencies []Lockable + delegation_map map[NodeID]GraphNode } func (state * BaseLockableState) MarshalJSON() ([]byte, error) { @@ -42,6 +48,16 @@ func (state * BaseLockableState) MarshalJSON() ([]byte, error) { dependency_ids[i] = dependency.ID() } + delegations := map[NodeID]*NodeID{} + for resource_id, node := range(state.delegation_map) { + if node == nil { + delegations[resource_id] = nil + } else { + str := node.ID() + delegations[resource_id] = &str + } + } + var owner_id *NodeID = nil if state.owner != nil { new_str := state.owner.ID() @@ -53,11 +69,13 @@ func (state * BaseLockableState) MarshalJSON() ([]byte, error) { Owner *NodeID `json:"owner"` Dependencies []NodeID `json:"dependencies"` Requirements []NodeID `json:"requirements"` + Delegations map[NodeID]*NodeID `json:"delegations"` }{ Name: state.name, Owner: owner_id, Dependencies: dependency_ids, Requirements: requirement_ids, + Delegations: delegations, }) } @@ -67,19 +85,31 @@ func (state * BaseLockableState) Name() string { // Locks cannot be passed between base lockables, so the answer to // "who used to own this lock held by a base lockable" is always "nobody" -func (state * BaseLockableState) OriginalLockHolder(id NodeID) GraphNode { - return nil +func (state * BaseLockableState) ReturnLock(resource_id NodeID) GraphNode { + node, exists := state.delegation_map[resource_id] + if exists == false { + panic("Attempted to take a get the original lock holder of a resource we don't own") + } + delete(state.delegation_map, resource_id) + return node } // Nothing can take a lock from a base lockable either -func (state * BaseLockableState) AllowedToTakeLock(id NodeID) bool { +func (state * BaseLockableState) AllowedToTakeLock(node_id NodeID, resource_id NodeID) bool { + _, exists := state.delegation_map[resource_id] + if exists == false { + panic ("Trying to give away lock we don't own") + } return false } -func (state * BaseLockableState) RecordLockHolder(id NodeID, lock_holder GraphNode) { - if lock_holder != nil { - panic("Attempted to delegate a lock to a lockable") +func (state * BaseLockableState) RecordLockHolder(resource_id NodeID, lock_holder GraphNode) { + _, exists := state.delegation_map[resource_id] + if exists == true { + panic("Attempted to lock a resource we're already holding(lock cycle)") } + + state.delegation_map[resource_id] = lock_holder } func (state * BaseLockableState) Owner() GraphNode { @@ -119,6 +149,7 @@ func NewLockableState(name string) BaseLockableState { owner: nil, requirements: []Lockable{}, dependencies: []Lockable{}, + delegation_map: map[NodeID]GraphNode{}, } } @@ -240,7 +271,7 @@ func UnlockLockable(ctx * GraphContext, lockable Lockable, node GraphNode, node_ return nil, nil, fmt.Errorf("Lockable %s failed to unlock: %e", lockable.ID(), lock_err) } - new_owner := node_state.OriginalLockHolder(lockable.ID()) + new_owner := node_state.ReturnLock(lockable.ID()) lockable_state.SetOwner(new_owner) err := lockable.Unlock(node, lockable_state) if err != nil { @@ -263,6 +294,7 @@ func LockLockable(ctx * GraphContext, lockable Lockable, node GraphNode, node_st if node == nil || lockable == nil { panic("Cannot lock without a specified node and lockable") } + ctx.Log.Logf("resource", "LOCKING: %s from %s", lockable.ID(), node.ID()) _, err := UpdateStates(ctx, []GraphNode{lockable}, func(states []NodeState) ([]NodeState, interface{}, error) { if lockable.ID() == node.ID() { @@ -276,10 +308,10 @@ func LockLockable(ctx * GraphContext, lockable Lockable, node GraphNode, node_st var lock_pass_allowed bool = false if lockable_state.Owner().ID() == lockable.ID() { - lock_pass_allowed = lockable_state.AllowedToTakeLock(node.ID()) + lock_pass_allowed = lockable_state.AllowedToTakeLock(node.ID(), lockable.ID()) } else { tmp, _ := UseStates(ctx, []GraphNode{lockable_state.Owner()}, func(states []NodeState)(interface{}, error){ - return states[0].(LockHolderState).AllowedToTakeLock(node.ID()), nil + return states[0].(LockHolderState).AllowedToTakeLock(node.ID(), lockable.ID()), nil }) lock_pass_allowed = tmp.(bool) } @@ -318,7 +350,7 @@ func LockLockable(ctx * GraphContext, lockable Lockable, node GraphNode, node_st old_owner := lockable_state.Owner() lockable_state.SetOwner(node) - node_state.RecordLockHolder(node.ID(), old_owner) + node_state.RecordLockHolder(lockable.ID(), old_owner) if old_owner == nil { ctx.Log.Logf("lockable", "LOCKABLE_LOCK: %s locked %s", node.ID(), lockable.ID()) diff --git a/thread.go b/thread.go index 8e9d24a..556f04e 100644 --- a/thread.go +++ b/thread.go @@ -5,7 +5,6 @@ import ( "time" "errors" "reflect" - "sync" "encoding/json" ) @@ -82,8 +81,6 @@ type BaseThreadState struct { parent Thread children []Thread child_info map[NodeID] ThreadInfo - resources map[NodeID]Lockable - delegation_map map[NodeID]GraphNode info_type reflect.Type } @@ -99,27 +96,35 @@ func (state * BaseThreadState) MarshalJSON() ([]byte, error) { parent_id = &new_str } - resources := map[NodeID]*NodeID{} - for _, resource := range(state.resources) { - original_owner := state.delegation_map[resource.ID()] - if original_owner != nil { - owner := original_owner.ID() - resources[resource.ID()] = &owner - } else { - resources[resource.ID()] = nil - } + requirements := make([]NodeID, len(state.requirements)) + for i, requirement := range(state.requirements) { + requirements[i] = requirement.ID() + } + + dependencies := make([]NodeID, len(state.dependencies)) + for i, dependency := range(state.dependencies) { + dependencies[i] = dependency.ID() + } + + delegations := map[NodeID]NodeID{} + for lockable_id, node := range(state.delegation_map) { + delegations[lockable_id] = node.ID() } return json.Marshal(&struct{ Name string `json:"name"` Parent *NodeID `json:"parent"` Children map[NodeID]interface{} `json:"children"` - Lockables map[NodeID]*NodeID `json:"resources"` + Dependencies []NodeID `json:"dependencies"` + Requirements []NodeID `json:"requirements"` + Delegations map[NodeID]NodeID `json:"delegations"` }{ Name: state.Name(), Parent: parent_id, Children: children, - Lockables: resources, + Dependencies: dependencies, + Requirements: requirements, + Delegations: delegations, }) } @@ -224,29 +229,6 @@ func LinkThreads(ctx * GraphContext, thread Thread, child Thread, info ThreadInf return nil } -// Threads allow locks to pass to their requirements, but they won't allow cycles -func (state * BaseThreadState) OriginalLockHolder(id NodeID) GraphNode { - node, exists := state.delegation_map[id] - if exists == false { - panic("Attempted to take a get the original lock holder of a resource we don't own") - } - delete(state.delegation_map, id) - return node -} - -func (state * BaseThreadState) AllowedToTakeLock(id NodeID) bool { - return false -} - -func (state * BaseThreadState) RecordLockHolder(id NodeID, lock_holder GraphNode) { - _, exists := state.delegation_map[id] - if exists == true { - panic("Attempted to lock a resource we're already holding(lock cycle)") - } - - state.delegation_map[id] = lock_holder -} - // Thread is the interface that thread tree nodes must implement type Thread interface { GraphNode @@ -333,11 +315,6 @@ type ThreadHandlers map[string]ThreadHandler type BaseThread struct { BaseNode - resources_lock sync.Mutex - children_lock sync.Mutex - info_lock sync.Mutex - parent_lock sync.Mutex - Actions ThreadActions Handlers ThreadHandlers @@ -414,10 +391,8 @@ var ThreadCancel = func(ctx * GraphContext, thread Thread, signal GraphSignal) ( func NewBaseThreadState(name string) BaseThreadState { return BaseThreadState{ BaseLockableState: NewLockableState(name), - delegation_map: map[NodeID]GraphNode{}, children: []Thread{}, child_info: map[NodeID]ThreadInfo{}, - resources: map[NodeID]Lockable{}, parent: nil, } } diff --git a/thread_test.go b/thread_test.go index d06921d..0eb2d16 100644 --- a/thread_test.go +++ b/thread_test.go @@ -3,6 +3,8 @@ package graphvent import ( "testing" "time" + "encoding/json" + "fmt" ) func TestNewEvent(t * testing.T) { @@ -12,10 +14,19 @@ func TestNewEvent(t * testing.T) { fatalErr(t, err) go func(thread Thread) { - time.Sleep(1*time.Second) + time.Sleep(10*time.Millisecond) SendUpdate(ctx, t1, CancelSignal(nil)) }(t1) err = RunThread(ctx, t1) fatalErr(t, err) + + _, err = UseStates(ctx, []GraphNode{t1}, func(states []NodeState) (interface{}, error) { + ser, err := json.MarshalIndent(states, "", " ") + fatalErr(t, err) + + fmt.Printf("\n%s\n", ser) + + return nil, nil + }) }