gnuradio / gr-bokehgui

Web based display for GNU Radio applications
GNU General Public License v3.0
82 stars 26 forks source link

Refreshing page causes endless loop and crashes #52

Open MiddleMan5 opened 2 years ago

MiddleMan5 commented 2 years ago

I have tried multiple versions of this library, and they all have a problem when multiple windows are open (or a page is refreshed). This makes it impossible to serve a gnuradio for multiple clients which was a big use case for us.

Versions: python: 3.10 node: 14.19.3-deb-1nodesource1 bokeh: 2.4.3 tornado: 6.1 gr-bokehgui: master (2be3889a3bda34427889654f0700cc4c547a2e05) gnuradio: 3.10.1.1-2

Please note: This issue was present in 3.8 and the maint-3.8 branch as well

Error Message:

ERROR:bokeh.util.tornado:Error thrown from periodic callback:
ERROR:bokeh.util.tornado:Traceback (most recent call last):
  File "/usr/local/lib/python3.10/dist-packages/tornado/gen.py", line 526, in callback
    result_list.append(f.result())
  File "/usr/local/lib/python3.10/dist-packages/bokeh/server/session.py", line 95, in _needs_document_lock_wrapper
    result = func(self, *args, **kwargs)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/server/session.py", line 229, in with_document_locked
    return func(*args, **kwargs)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/document/callbacks.py", line 450, in wrapper
    return invoke_with_curdoc(doc, invoke)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/document/callbacks.py", line 408, in invoke_with_curdoc
    return f()
  File "/usr/local/lib/python3.10/dist-packages/bokeh/document/callbacks.py", line 449, in invoke
    return f(*args, **kwargs)
  File "/usr/local/lib/python3.10/dist-packages/bokehgui/waterfall_sink_c.py", line 89, in callback
    self.update( )
  File "/usr/local/lib/python3.10/dist-packages/bokehgui/waterfall_sink_c.py", line 105, in update
    self.waterfall_renderer[i].latest = list(output_items[i])
  File "/usr/local/lib/python3.10/dist-packages/bokeh/core/has_props.py", line 230, in __setattr__
    return super().__setattr__(name, value)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/core/property/descriptors.py", line 285, in __set__
    self._set(obj, old, value, setter=setter)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/core/property/descriptors.py", line 559, in _set
    self._trigger(obj, old, value, hint=hint, setter=setter)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/core/property/descriptors.py", line 637, in _trigger
    obj.trigger(self.name, old, value, hint, setter)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/model/model.py", line 567, in trigger
    super().trigger(descriptor.name, old, new, hint=hint, setter=setter)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/util/callback_manager.py", line 194, in trigger
    self.document.callbacks.notify_change(cast(Model, self), attr, old, new, hint, setter, invoke)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/document/callbacks.py", line 236, in notify_change
    self.trigger_on_change(event)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/document/callbacks.py", line 378, in trigger_on_change
    invoke_with_curdoc(doc, invoke_callbacks)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/document/callbacks.py", line 408, in invoke_with_curdoc
    return f()
  File "/usr/local/lib/python3.10/dist-packages/bokeh/document/callbacks.py", line 377, in invoke_callbacks
    cb(event)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/document/callbacks.py", line 263, in <lambda>
    self._change_callbacks[receiver] = lambda event: event.dispatch(receiver)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/document/events.py", line 400, in dispatch
    super().dispatch(receiver)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/document/events.py", line 223, in dispatch
    cast(DocumentPatchedMixin, receiver)._document_patched(self)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/server/session.py", line 247, in _document_patched
    raise RuntimeError("_pending_writes should be non-None when we have a document lock, and we should have the lock when the document changes")
RuntimeError: _pending_writes should be non-None when we have a document lock, and we should have the lock when the document changes

ERROR:bokeh.util.tornado:Error thrown from periodic callback:
ERROR:bokeh.util.tornado:Traceback (most recent call last):
  File "/usr/local/lib/python3.10/dist-packages/tornado/gen.py", line 526, in callback
    result_list.append(f.result())
  File "/usr/local/lib/python3.10/dist-packages/bokeh/server/session.py", line 95, in _needs_document_lock_wrapper
    result = func(self, *args, **kwargs)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/server/session.py", line 229, in with_document_locked
    return func(*args, **kwargs)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/document/callbacks.py", line 450, in wrapper
    return invoke_with_curdoc(doc, invoke)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/document/callbacks.py", line 408, in invoke_with_curdoc
    return f()
  File "/usr/local/lib/python3.10/dist-packages/bokeh/document/callbacks.py", line 449, in invoke
    return f(*args, **kwargs)
  File "/usr/local/lib/python3.10/dist-packages/bokehgui/waterfall_sink_c.py", line 89, in callback
    self.update( )
  File "/usr/local/lib/python3.10/dist-packages/bokehgui/waterfall_sink_c.py", line 105, in update
    self.waterfall_renderer[i].latest = list(output_items[i])
  File "/usr/local/lib/python3.10/dist-packages/bokeh/core/has_props.py", line 230, in __setattr__
    return super().__setattr__(name, value)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/core/property/descriptors.py", line 285, in __set__
    self._set(obj, old, value, setter=setter)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/core/property/descriptors.py", line 559, in _set
    self._trigger(obj, old, value, hint=hint, setter=setter)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/core/property/descriptors.py", line 637, in _trigger
    obj.trigger(self.name, old, value, hint, setter)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/model/model.py", line 567, in trigger
    super().trigger(descriptor.name, old, new, hint=hint, setter=setter)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/util/callback_manager.py", line 194, in trigger
    self.document.callbacks.notify_change(cast(Model, self), attr, old, new, hint, setter, invoke)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/document/callbacks.py", line 236, in notify_change
    self.trigger_on_change(event)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/document/callbacks.py", line 378, in trigger_on_change
    invoke_with_curdoc(doc, invoke_callbacks)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/document/callbacks.py", line 408, in invoke_with_curdoc
    return f()
  File "/usr/local/lib/python3.10/dist-packages/bokeh/document/callbacks.py", line 377, in invoke_callbacks
    cb(event)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/document/callbacks.py", line 263, in <lambda>
    self._change_callbacks[receiver] = lambda event: event.dispatch(receiver)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/document/events.py", line 400, in dispatch
    super().dispatch(receiver)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/document/events.py", line 223, in dispatch
    cast(DocumentPatchedMixin, receiver)._document_patched(self)
  File "/usr/local/lib/python3.10/dist-packages/bokeh/server/session.py", line 247, in _document_patched
    raise RuntimeError("_pending_writes should be non-None when we have a document lock, and we should have the lock when the document changes")
RuntimeError: _pending_writes should be non-None when we have a document lock, and we should have the lock when the document changes
Notou commented 2 years ago

Hummm, yes, I do see this error message. The surprising thing is that I recall trying opening multiple windows before (same version) without this issue.

This will need to be investigated.

Notou commented 2 years ago

It would seem that only the waterfall sink (complex and float), and label blocks cause this error. Unfortunately, I won't be able to look into it more in the next week or so.

Notou commented 2 years ago

Skip to the end for the workaround It may not seem like it, but I did not completely forget about the issue, and I believe I found where the problem is. I'll try do describe it here for future reference:

Currently, the bokehgui code for one plot is separated in 2 (one could even say 3) components:

When a user connects, the bokeh server create a brand new session for that user, and asks the bokehgui python code to populate a blank document with plots and widgets to display on the user's browser. At that time, the python code creates a new instance of the bokeh plot object (plus, if applicable, an object to stream it data), stores the plot object into its plot attribute, and register a callback to be called at regular intervals (the update period). Most plots have it so the callback function contains explicit reference to the plot object (and its data stream), so it does not need to refer to the self.plotattribute. But the waterfall does not. It thus tries to feed new data to whatever object is currently stored. And, when a second session is created (by reloading the page, or opening a new one), the python code only stores the most recent one, but the callback of the older one keeps being called (at least for a while), so it tries to feed data to the wrong object, without having acquired the lock for it, hence the error.

The most basic fix should be to have the callback for the waterfall take the correct plot object as argument, like the other plots do. But that would not solve the following underlying problem: When a display parameter needs to be changed from the gnuradio side, like for instance the x axis of a frequency plot when center frequency is changed, the python code can only operate on the plot object it has stored, so only the latest opened tab would see it. For that, the python code would need to store all the created bokeh plot objects, and a way to remove them from storage when the related session gets destroyed so we don't keep on piling up objects.

I believe I know how to do that, and it should not take too long. But in the mean time, there actually is a workaround: Since the error is created when multiple sessions exists, one just need to make sure only one session is ever created. For that, you need to supply a session id in the url you connect to for all browser windows, such as: http://localhost:5006/?bokeh-session-id=test_bokehgui Here, you can put whatever string in place of test_bokehgui.