intuit / QuickBooks-V3-Java-SDK

Java SDK for QuickBooks REST API v3 services
Apache License 2.0
70 stars 142 forks source link

Config is not thread safe (uses a ThreadLocal) #170

Open voidmain opened 3 years ago

voidmain commented 3 years ago

While working on a long running piece of code that reconciles between Stripe and QBO, I started a separate thread that calls the existing service code I wrote. When I did this I noticed that it started failing. This was due to the fact that com.intuit.ipp.util.Config uses a ThreadLocal to store configuration changes. While normally, this code worked fine because the service had already configured the QBO SDK, once it was invoked from a separate thread, it started failing. I would not expect that an object previously instantiated and working fine would suddenly stop working when called from another thread.

Here's some sample code:

MyQBOService service = makeService();
service.reconcile(); // This works fine

Thread thread = new Thread(service);
thread.start(); // This doesn't work because they configuration setup previously is erased

This makes the Java SDK non-thread safe without requiring explicit re-configuration prior to pushing logic to a separate thread.

In general, using ThreadLocals is somewhat an anti-pattern that should be avoided, or at least provide a method to allow developers in highly multi-threaded environments to configure everything inline. My preferred method of handling this would have been to pass a Config object into the DataService constructor or the Context constructor.

mattluce commented 3 months ago

Any updates on this?