deephaven / streamlit-deephaven

Deephaven Streamlit custom component
MIT License
1 stars 3 forks source link

`start_server` is not thread safe #8

Closed niloc132 closed 3 months ago

niloc132 commented 4 months ago

With more than one browser window open automatically trying to re-run code, it is possible to get into a race condition with start_server where two servers are starting at the same time. This results in an error message:

 Traceback (most recent call last):
   File "/usr/local/lib/python3.10/site-packages/streamlit/runtime/scriptrunner/script_runner.py", line 584, in _run_script
     exec(code, module.__dict__)
   File "/src/app.py", line 28, in <module>
     app()
   File "/src/app.py", line 8, in app
     start_server(jvm_args=[
   File "/usr/local/lib/python3.10/site-packages/streamlit_deephaven/__init__.py", line 42, in start_server
     s = Server(host=host, port=port, jvm_args=jvm_args, dh_args=dh_args)
   File "/usr/local/lib/python3.10/site-packages/deephaven_server/server.py", line 55, in __init__
     self.j_server = jpy.get_type('io.deephaven.python.server.EmbeddedServer')(host, port, dh_args)
 RuntimeError: java.lang.IllegalStateException: Should only LogBufferGlobal#setInstance once
         at io.deephaven.io.logger.LogBufferGlobal.setInstance(LogBufferGlobal.java:16)
         at io.deephaven.server.runner.MainHelper.init(MainHelper.java:112)
         at io.deephaven.python.server.EmbeddedServer.<init>(EmbeddedServer.java:88)

With two concurrent calls to start_server, both will pass the Server.instance is None check and attempt to create their own instance at the same time, which is not allowed. Instead, there should probably be a lock guarding this creation.

This only happens on startup, after the server has been created the Server.instance is None check cannot pass again, and later use/rerun will never fail - so this is mostly noise on startup, but still is incorrect.