possible thread safety issue about cs_signal HOT 3 CLOSED

Thank you for working on integrating CsSignal with your benchmark suite. I looked at your benchmark and I could not quite tell the usage pattern.

CsSignal is designed to support thread-aware signal delivery, however you will need to override the implementation of compareThreads and queueSlot in CsSignal::SlotBase. Without these overrides, signal activations will never be queued and slots will be invoked immediately in the emitting thread. Since CopperSpice is a client of CsSignal which provides its own threading model, CopperSpice applications do not need to supply these overrides.

I am also not sure whether this issue is in CsSignal or something related to the way your benchmark is using the library. In attempting to reproduce it, I ran into some issues building from your rework branch.

There is a reference to benchmark_spp.cpp in the CMakeLists.txt, and this file is not present in the repo. Commenting this line out allows the CMake process to continue, but there were build errors, attached below. This build log is from my Debian 10 system, using CMake 3.13.2, ninja 1.8.2 and gcc 8.2.0.

signal-slot-benchmarks-buildlog.txt

Unfortunately, I do not recognize most of the errors in this log. The ones in CsSignal look like they might be related to the LIB_SIG_EXPORT macro not being defined for some reason.

If you let me know your exact build environment, I will see if I can reproduce this issue and if I can help debug this.

from cs_signal.

Hello, I'm currently using a single dev environment of visualstudio 2017 and currently lack an environment suitable for gcc testing. Thanks for reporting the cmake list issues, I will definitely need to remove that old nano-signal-slot lib, its time has come.

I've integrated cs_signal using the default "AutoConnection" connection kind and it seems to be passing the validation method used to make sure all integrated libs are functioning "correctly" in regards to slot emission.

So I think the issue now is my integration only supports single threaded due to not suitably overriding queueSlot. (although I might have tried copying from the peach test with the same result. the include is still in there for deque)

I'll try again later today after work.

from cs_signal.

I have elected to only integrate cs_signal to a level only suitable for the single threaded benchmarks. If the signal-slot-benchmarks project gets a copperspice fork in the future, I would gladly integrate changes.

Here is the revision for reference.

Thanks for reporting those cmake issues again, it "should" be straightened out now.

from cs_signal.