Locks + callbacks

[aakn]

If I have an interface which has two functions

1. Function that takes a callback, (think transactions)
2. Function that does some work

And then if I call function two from the callback, it leads to a deadlock.

I've attached a simple reproduction of the issue. simple interface: https://gist.github.com/aakn/f41ea322cb77bbed1b9e79926e98db88 test: https://gist.github.com/aakn/346ee11fb7798592e6fbadebdc38b5f1

Note: this issue was introduced while fixing this issue: https://github.com/petergtz/pegomock/issues/55 Feel free to ask me any questions to help clarify the issue.

[aakn]

I've worked around this by using two different instances of MockStorage so that the locks are isolated. Do you reckon there's a better way of solving this?

[petergtz]

Hi @aakn,

I think you're right, that deadlock should probably not happen. But I need to check if that is true, by closely examining the code paths. You're also right that it was introduced with #55. Maybe we can get rid of the problem by reducing the scope that the Mutex holds to just the scope where we actually change internal state of the mock, and maybe that resolves your problem.

That being said, I was wondering if what you have in one object/one interface should really be in one. Or if it semantically makes more sense to separate those, just like you did by using separate mocks. The responsibility of saving seems separate from the logic in DoTransaction. Further wondering if DoTransaction ever has different implementations. But obviously, it's hard to judge, because your real use-case might have other constraints that are not visible in your simplified example.

So, long story short, I'll see if we can get rid of the deadlock, by changing the Mutex. I'll try to look at it this or next week.

[aakn]

Thanks @petergtz

There's just one implementation for DoTransaction. They are together now because we want to be able to do nested transactions. So we went with DoTransaction as a function which returns a new storage object with a new database transaction. The other functions in the Storage interface provide database operations to be run in the same transaction.

So, long story short, I'll see if we can get rid of the deadlock, by changing the Mutex. I'll try to look at it this or next week. Thank you. 😄 This will work.

[petergtz]

@aakn Can you check if the latest commit fixes your issue?

[aakn]

Sorry, haven't gotten around to testing it. Will do it by tomorrow.

[aakn]

The fix worked! Thanks, Peter.