Deadlock when calling IsGranted from within Walk

[shmul]

Hi,

I wrote a simple function to test my use of the package and discovered that if one calls IsGranted from within the Walk handler function, the code hangs. Looking into the code it looks like a deadlock.
Would it be possible to expose a non locking version of IsGranted (basically the existing private isGranted).

Cheers,
Shmul

[mikespook]

Could you show me some more codes to help me understand the use case?
I'd like to modify the helper functions rather than RBAC API, but with the use case, we may figure out which way is the best one for us.

Thanks & cheers!

[ccamel]

@mikespook
I confirm that there is a possible deadlock when calling a RBAC function inside the Walk handler.
In go, rw mutex are not reentrant.

I don't have a very useful case, but here's a unit test which shows the issue.

func TestRBACDeadLock(t *testing.T) {
	// Arrange
	rbac := gorbac.New()
	rA := gorbac.NewStdRole("role-a")
	pA := gorbac.NewStdPermission("permission-a")
	rA.Assign(pA)
	rbac.Add(rA)

	// Act
	gorbac.Walk(rbac, func(role gorbac.Role, strings []string) error {
		// -> boom: deadlock!
		rbac.Add(gorbac.NewStdRole("role-b"))

		return nil
	})
}

[mikespook]

@ccamel Thx!

Yes, it's a known issue we have. I'm considering to expose the lock method and let users control the lock with helper functions. But I haven't found a solution which could be easy but safe to do so.

[ccamel]

@mikespook Thanks for your reply.

I for one don't really like the idea to expose the lock object to outside. I would prefer to have the callback function called in a context freed from any lock. It should be feasible but at the cost of a copy of the context before the call.

[mikespook]

Yes, I don't like it either. That's the reason I didn't change the API when I found the deadlock.
Another issue is, helper functions are not the core functions for the library, thus changing the main API to solve an issue in helper functions sounds not a practical way.

Exposing the lock-free functions would be the easiest way, but I'm wondering the effects to the helper functions when it's calling from a concurrent situation. Let's think about it. I will try to figure out all possibility use-case to see if it's good or not.