gutter icon indicating copy to clipboard operation
gutter copied to clipboard

Multithreading

Open teeberg opened this issue 9 years ago • 9 comments

I'm using a django LiveServerTestCase that runs a server in a second thread. Because the tests and the server run in two different threads now, I can't use gutter switches anymore since the gutter Manager inherits from threading.local. Are there plans for adding support for multiple threads?

Whatever the plans are, it would be nice to mention this limitation more prominently in the README. :-)

teeberg avatar May 02 '15 23:05 teeberg

Hi @teeberg,

Gutter is backed by durabledict, which should give you a consistent[1] view of switches in your data store, even if there is a copy of the manger per thread. Was there a specific issue or some other problem you were running into that was unexpected with multiple threads.

[1] Obviously the memory backend will not be consistent, but the Redis and ZooKeeper ones should be consistent enough no problem.

Fluxx avatar May 04 '15 15:05 Fluxx

Hi @Fluxx, thanks for the fast answer! My understanding of gutter was clearly not good enough to correctly identify the problem. :-) We are in fact using a memory backend in our test environment. When we settled for it, we didn't worry about multiple threads. I also didn't realise this would cause a problem. Thanks for clarifying!

teeberg avatar May 04 '15 16:05 teeberg

I've debugged some more and tracked down what I think is the real problem: What really doesn't work in the two-thread scenario is the SwitchContextManager from gutter.client.testutils. It temporarily overrides gutter.active, but because the gutter Manager is thread-local, the other thread doesn't pick up this change. Making the Manager inherit from object instead of threading.local fixes the problem, but that can't be the solution. :-)

teeberg avatar May 04 '15 22:05 teeberg

Ah yeah. That's a limitation of SwitchContextManager since, as you mentioned, it monkey patches gutter.active.

IIRC (as I wrote that code over 3 years ago), the intention with making the Manager a threading.local was to prevent people from shooting themselves in the foot by sharing a Manager instance as a singleton between threads and causing problems. I still agree with that sentiment I think, but I do hear you that your use case of the Django LiveServerTestCase would be nice to have thread communication on the Manager state.

I think what might be best in this scenario is to change the class hierarchy a little to have a base somewhat private BaseManager class not inherit from theading.local, and instead make the canonical Manager class inherit from (BaseManager, threading.local). Thus for most users they can just use the canonical Manager class, but for users who need to some more savvy thread coordination they can use a custom manager they inherit from BaseManager.

Thoughts?

Fluxx avatar May 04 '15 22:05 Fluxx

I think it's reasonable to make it thread-local by default. It would of course be nice if the Manager was fully thread-safe but I'm not sure how hard that would be to achieve.

Leaving that out, what you're suggesting sounds like a quick solution that I could work with! It would be awesome if you could make this change here! Maybe other users can benefit from it too, then :-)

teeberg avatar May 04 '15 22:05 teeberg

It would be awesome if you could make this change here! Maybe other users can benefit from it too, then :-)

Yeah I can totally do that. Won't be today though, but by the weekend for sure.

Fluxx avatar May 05 '15 19:05 Fluxx

Awesome, I'm looking forward to that change! Thank you so much for your efforts!

teeberg avatar May 05 '15 19:05 teeberg

How do you feel about a patch like this one? https://github.com/teeberg/gutter/commit/b887062093d1297b0e30c201ab050bc03d8da8f8

teeberg avatar Jun 20 '17 22:06 teeberg

That seems like a good idea. I'll be taking a look at this repo soon.

thekashifmalik avatar Jun 20 '17 22:06 thekashifmalik