msgraph-sdk-python-core icon indicating copy to clipboard operation
msgraph-sdk-python-core copied to clipboard

Thread unsafe GraphClient

Open rsimac opened this issue 1 year ago • 1 comments

For those of you still sticking with version 0.2.2 of this library due to its continuous 'preview' phase, just heads up this lib is not thread safe. As in, do not call the GraphClient from multiple threads, and expect to have the thread specific own copy...

Due to this library use of class static variable __instance as visible below: https://github.com/microsoftgraph/msgraph-sdk-python-core/blob/d14eda5bafaf548154c9472e765599beeb9c04f9/msgraph/core/_graph_client.py#L74

when called from different threads, using different credentials, the creating of new GraphClient will return the old graph client instance that was created in the first thread that called it, thus rendering it unusable for the thread that called it right now.

This can be reproduced by very simple program, displaying the thread ID and the physical address of the graph client object, showing the exact same object (address) from both threads, even if the credentials supplied were completely different.

def simple_thread(name, credentials):

    while True:
        
        msgraph_client = GraphClient(credential=credentials) 

        logger.info(f"thread: {name}, client: {msgraph_client}")

        time.sleep(2)

    





if __name__ == '__main__':

    cloud_account_1 = {'azure_ad_id': "*******",
                     'azure_app_id': "**********",
                     'azure_app_key': "***********"}

    credentials_msgraph_T1 = ClientSecretCredential(tenant_id=cloud_account_1.get('azure_ad_id'), client_id=cloud_account_1.get('azure_app_id'), 
                                                 client_secret=cloud_account_1.get('azure_app_key'))    

    t1 = threading.Thread(target=simple_thread, args=("T1", credentials_msgraph_T1))
    t1.daemon=True
    t1.start()


    cloud_account_2 = {'azure_ad_id': "***********",
                     'azure_app_id': "************",
                     'azure_app_key': "***************"}


    credentials_msgraph_T2 = ClientSecretCredential(tenant_id=cloud_account_2.get('azure_ad_id'), client_id=cloud_account_2.get('azure_app_id'), 
                                                 client_secret=cloud_account_2.get('azure_app_key'))    

    t2 = threading.Thread(target=simple_thread, args=("T2", credentials_msgraph_T2))
    t2.daemon=True
    t2.start() 


    #wait for above to exit
    t1.join()
    t2.join()

    logger.info("Exiting main program")   

Output from above program, showing exact same client returned from both threads, even if the credentials were totally different...

INFO:root:thread: T1, client: <msgraph.core._graph_client.GraphClient object at 0x000002278D779400>
INFO:root:thread: T2, client: <msgraph.core._graph_client.GraphClient object at 0x000002278D779400>
INFO:root:thread: T1, client: <msgraph.core._graph_client.GraphClient object at 0x000002278D779400>
INFO:root:thread: T1, client: <msgraph.core._graph_client.GraphClient object at 0x000002278D779400>
INFO:root:thread: T2, client: <msgraph.core._graph_client.GraphClient object at 0x000002278D779400>

rsimac avatar Jun 01 '23 09:06 rsimac

Until this issue gets addressed :) this is the workaround that worked for me. I just override the problematic __new__() method, and avoided all that __instance business, returning plain new fresh class instance at each instantiation... Code sample is below, it is tested and now everything works as expected, each caller gets its own class instance from its own thread...

I don'h have bandwidth right now but it would be wise to check if this line of thinking/coding (singletons?) has propagated to any 'new' implementations of this client library...

Code workaround sample, using ThreadSafeGraphClient in place of GraphClient:

class ThreadSafeGraphClient(GraphClient):
    '''
    Workaround for issue #374 overriding and fixing thread unsafe msgraph api class init 
    Everything else remains native msgraph
    Bug also filed under: https://github.com/microsoftgraph/msgraph-sdk-python-core/issues/245
    '''
    def __new__(cls, *args, **kwargs):
        ret = object.__new__(cls)
        return ret
        


def simple_thread(name, credentials, account_name):

    while True:

        msgraph_client = ThreadSafeGraphClient(credential=credentials)

        logger.info(f"thread: {name}, client: {msgraph_client}")

        r = msgraph_client.get(f"/users/{account_name}")

        logger.info(f"Returned {r.json()}")


        time.sleep(2)

rsimac avatar Jun 02 '23 07:06 rsimac

thanks for reporting this issue, we have since released a new version of the SDK which this issue doesn't apply to. We encourage you to migrate to the new version and open a new issue if you still need help

baywet avatar Apr 25 '24 15:04 baywet