syrusakbary/promise

Thread safety using 2.0

rslinckx opened this issue · 2 comments

The new version introduces the concept of event loop, and uses a Context class to be able to chain contexts.

There is however a global variable "context_stack" which is accessed when pushing/popping contexts.

I think this will lead to errors if i use for example gevent, or event regular threads, the global variable will get messed up across threads.

Frameworks usually use some kind of threadlocal context stack (such as flask) or explicit stack management to be able to work when using threads.

I think the actual implementation will just break any thread of greenletted application using it?

I haven't had a chance to really troubleshoot it yet, but when I tried to upgrade from promises 1.0 to 2.0 with my graphql-subscriptions implementation, most of the subscription_manager tests fail, specifically when promises are called inside of a greenlet. Latest tests commit here. When I debugged with pdb and tried Promise.resolve inside the same greenlet that was failing...I noticed in 1.0.1 they were being fulfulled almost immediately, but in 2.02, they stayed in a "pending" status.

This is now fixed in master.