sccn / lsl_archived

Multi-modal time-synched data transmission over local network
242 stars 133 forks source link

pylsl and PyQt - threading on Unix #324

Closed cbrnr closed 6 years ago

cbrnr commented 6 years ago

I wanted to use pylsl in a simple PyQt-based online visualization app I'm currently developing with @Yida-Lin. According to the corresponding README.rst, "on Linux one currently cannot call pylsl functions from a thread that is not the main thread" (actually, this is also true for macOS, so I assume this is an issue for Unix-like platforms).

This is a no-go for a GUI application, where the data collection should be performed in a separate thread to avoid blocking the main GUI thread. Therefore, I was wondering what exactly causes this problem and can it somehow be fixed (in pylsl) or worked around (in the PyQt app)? I have had a similar problem with PyQt and spawning a subprocess, which did not work on Unix platforms unless I set the spawning method to multiprocessing.set_start_method("spawn").

For reference, here's the error I get when I try to run the app on macOS or Linux:

Traceback (most recent call last):
  File "/Users/clemens/Repositories/SigVisualizer/paintwidget.py", line 43, in run
    chunk, timestamps = self.inlet.pull_chunk(max_samples=self.chunkSize)
  File "/Users/clemens/anaconda/lib/python3.6/site-packages/pylsl/pylsl.py", line 844, in pull_chunk
    handle_error(errcode)
  File "/Users/clemens/anaconda/lib/python3.6/site-packages/pylsl/pylsl.py", line 1134, in handle_error
    raise InternalError("an internal error has occurred.")
pylsl.pylsl.InternalError: an internal error has occurred.

If anyone wants to replicate, just clone the current master of https://github.com/Yida-Lin/SigVisualizer and run python sigvisualizer.py on macOS or Linux. On Windows, everything works fine.

tstenner commented 6 years ago

I don't know exactly why (I could replicate the problem, same error message, with the old version), but the new version with Chadwick's changes works for me.

cbrnr commented 6 years ago

I didn't know that there is a new version - thanks for pointing that out, this works on my Mac!

Where are liblsl and the related bindings currently developed? I see that there's activity in both https://github.com/labstreaminglayer/liblsl and https://github.com/sccn/labstreaminglayer, but only the former seems to work. Furthermore, pip install pylsl still pulls in the old version - any chance this can be updated to the working version?

cboulay commented 6 years ago

I did update it on pypi, at least I thought I did. What platform are you using to test pip install (OS and pay version) so I can reproduce getting the old version?

cbrnr commented 6 years ago

I'm on macOS 10.13.6, Python 3.7, and pip pulls pylsl 1.12.0. Same on Linux (all platforms are included in the binary wheel).

tstenner commented 6 years ago

It's a bit complicated - the whole discussion is in #273. Short: I (and Chadwick) push most of our changes to the new repositories and this repository should at some time in the future only contain links to these new repos. We will hopefully discuss how and when this will happen in september.

cbrnr commented 6 years ago

Thanks @tstenner, this makes sense.

cboulay commented 6 years ago

@cbrnr Well that is the new version... so I don't know what the difference is between that and when you got it to work on your Mac from the 'new' repo. When you installed using the new repo, do you know where it was searching for liblsl.dylib? BTW, I think this might be a more efficient conversation on Slack. If you can hop on there then I'd be happy to help in real-time.

cboulay commented 6 years ago

@tstenner I can reproduce the error using a self-built liblsl64.1.12.0.dylib (with lslboost or external) or with the dylib downloaded from bintray. However, the liblsl64.1.4.0.dylib from here (misleading URL) works fine.

Can you help me figure out how far back I should go to find a version of liblsl that is similar to whatever is in the Release page? If I can build my own that works, then we can figure out what has changed. I think it's necessary to build my own first in case it's not something in the library code itself, but something in my build configuration.

tstenner commented 6 years ago

I've pushed new commits to both the liblsl as well as the liblsl-Python repository to give better error messages. The new OS X library (already uploaded, same link) will give you better error messages what went wrong. Also, pylsl.library_info() will give us a better idea which liblsl is actually loaded.

If we had a one-line testcase (e.g. python -c 'import threading; import pylsl; thread.start(lambda: pylsl.create_outlet()), we could let git find the problem on its own.

cbrnr commented 6 years ago

pylsl.pull_chunks is causing the error, so maybe you can use that to create the one-liner?

cbrnr commented 6 years ago

Or rather, inlet.pull_chunk.

tstenner commented 6 years ago

I've found the problem in the 1.12 build (liblsl is already uploaded, but not pylsl), not sure about the threading issue though.

cbrnr commented 6 years ago

Here's the shortest example I could come up with to trigger the error:

import threading
from pylsl import StreamInlet, resolve_stream

streams = resolve_stream()
inlet = StreamInlet(streams[0])
threading.Thread(target=inlet.pull_chunk).start()
tstenner commented 6 years ago

Great, I've shortened it a bit to 'python -c 'import threading; import pylsl; inlet = pylsl.StreamInlet(pylsl.resolve_stream()[0]); threading.Thread(target=inlet.pull_chunk).start()'` (just in case we need to git bisect).

@cboulay could you see if it's working for you now and if yes, update the pylsl package?

cboulay commented 6 years ago

@tstenner I just built the latest liblsl on my machine and it worked well. Thanks for identifying and fixing the bug. (How did a missing function argument make it past compilation?)

Did you trigger a new build for all platforms? I would like to make sure the binaries are in the wheel are all the same version.

Also I think this needs a version bump to make sure anyone with pylsl 1.12.0 will be able to update to this fixed binary.

tstenner commented 6 years ago

@tstenner I just built the latest liblsl on my machine and it worked well. Thanks for identifying and fixing the bug. (How did a missing function argument make it past compilation?)

The next argument was a default argument so leaving it out just generates a warning with -Wall.

Did you trigger a new build for all platforms? I would like to make sure the binaries are in the wheel are all the same version.

OS X, 64 bit Linux, Windows 32 bit with VS2008 (for Python 2.7) and 64 with VS2015 (Python >3)

cboulay commented 6 years ago

pylsl has been updated on pypi.

@tstenner The latest Windows binaries in bintray are only the debug binaries, so I didn't use those and instead used self-compiled binaries, but that means only x32 for now.

@cbrnr Please test the new pylsl on pypi (1.12.1). If everything is OK then please close this issue.

tstenner commented 6 years ago

Release builds are up, I forgot that the --config switch doesn't propagate to Ninja builds. Would it make sense to use RelWithDebInfo for all builds so the error messages and stacktraces include the real function names?

cbrnr commented 6 years ago

Nope, this version doesn't even work for sending data (which worked before):

Traceback (most recent call last):
  File "LSL_sender.py", line 3, in <module>
    from pylsl import StreamInfo, StreamOutlet
  File "/usr/local/lib/python3.7/site-packages/pylsl/__init__.py", line 2, in <module>
    from .pylsl import IRREGULAR_RATE, DEDUCED_TIMESTAMP, FOREVER, cf_float32,\
  File "/usr/local/lib/python3.7/site-packages/pylsl/pylsl.py", line 1200, in <module>
    lib = CDLL(libpath)
  File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/ctypes/__init__.py", line 356, in __init__
    self._handle = _dlopen(self._name, mode)
OSError: dlopen(/usr/local/lib/python3.7/site-packages/pylsl/liblsl64.dylib, 6): no suitable image found.  Did find:
    /usr/local/lib/python3.7/site-packages/pylsl/liblsl64.dylib: file too short
    /usr/local/lib/python3.7/site-packages/pylsl/liblsl64.dylib: file too short
cboulay commented 6 years ago

The symlink was mangled when I copied from one computer to another on a FAT32 drive, and I was masking the issue from myself by using different virtual envs. Sorry! Please try again. 1.12.2

cbrnr commented 6 years ago

Wonderful! It's working now! Thanks a lot!