Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lvssh_extensions Multi-thread Safety and Memory Allocation #3

Open
j-medland opened this issue Apr 21, 2024 · 1 comment
Open

lvssh_extensions Multi-thread Safety and Memory Allocation #3

j-medland opened this issue Apr 21, 2024 · 1 comment

Comments

@j-medland
Copy link
Contributor

Hello - I have done quite a bit with C/C++ integration into LabVIEW and I am always interested to see what other folks are doing.

Thanks for contributing to the community and please don't feel obliged to respond to this issue. I am a firm believer that if you write code that solves your problem then that's great and if you share it with the community, even better. You don't owe anything else.

Now - my questions

Question 1
I was curious to know how you are managing multi-thread safety in your lvssh_extension code:

It looks like you declare some global variables to store pointers to memory and it appears from the LV-code that you could have multiple open sessions running which would all be accessing the same global variables from potentially (depending on the LabVIEW thread-managment) different threads.

Do you manage or protect against race conditions in some way (from the C-code side or the LabVIEW side)?

Question 2
You seem to be allocating onto the heap (with malloc) in a number of function calls with objects that are then passed to LVPostUserEvent.

Is there a particular reason for this - for situations where the allocated memory shares the scope of the function (i.e. you call free toward the end of the function), I think stack allocation is fine. For variables that are basic data types you can just decalre and intialize them and then use & to get their address which you pass to LVPostUserEvent.

Question 3
I have a rough CMake configuration that will handle building and installing the shared-library files for 32 and 64 bit Windows and 64-bit Linux if you are interested. It requires some modifications to the .h and .cpp files as you are using some Windows specific conventions and syntax. It is also capable of building libssh2 as a dependency and locating LabVIEW's cintools on those platforms.

Would you be interested in a PR with it in? This would be in addition to my current PR #2 and would also require you to undertake the testing as I don't quite know what would need to be done to ensure it works on the platforms specified.

Cheers

@logmanoriginal
Copy link
Owner

Thanks for taking your time to review the code! I'm always happy to receive feedback and suggestions for improvement.

As a side note, I typically work with LV or C#. Consequently, my C/C++ skills are very basic. Turns out reading C++ is MUCH easier than writing it 😭

Question 1: Thread-safety
This is a very good point. The extension library is not thread-safe and there are no precautions in the LabVIEW code. I have considered passing an object to the callback VI (for the return values) instead of using global variables but couldn't figure out a good way to make it work without getting lost in pointer hell on the LabVIEW side. This is one of the reasons why the extension library can globally be disabled; it is quite fragile.

Question 2: Heap allocation
There is no particular reason for this, other than being the only way I could figure out how to make it work. This might also explain the issues I saw after reviewing #2 regarding the heap corruption exceptions.

Question 3: CMake
I'd be happy to and can take care of testing. At least the Windows side as I currently don't have a functional Linux machine with LabVIEW installed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants