Segfault from destroying tags about libplctag HOT 7 CLOSED

Once I make the API threadsafe I would like to see if you can recreate this. At that point you should be able to stop using the tag lock/unlock API calls unless you are chaining multiple calls.

from libplctag.

It is a long log file :-)

I am trying to figure out what is happening here. There is a lot of threading going on and it looks like there are cases where the tag is passed among threads. The log file ends in the middle of output from the background IO thread, so I assume that is where the crash happened. However, before that there is some stuff that I cannot figure out because I do not know how your apps works.

In the library log entries, there is an indicator on each log line of which thread wrote the log. At the very end of the log file, thread 24 (the numbers are simply assigned in order that the threads are seen) is active. However, what it does seems odd.

Thread 24 first shows up here:

thread(0024) 2017-04-01 14:58:32.892 INFO plc_tag_unlock:321 Starting.
thread(0024) 2017-04-01 14:58:32.892 INFO plc_tag_unlock:331 Done.
thread(0024) 2017-04-01 14:58:32.892 INFO plc_tag_destroy:475 Starting.
thread(0024) 2017-04-01 14:58:32.892 INFO plc_tag_destroy_mapped:413 Starting.
thread(0024) 2017-04-01 14:58:32.892 WARN mutex_destroy:426 error while attempting to destroy mutex.
thread(0024) 2017-04-01 14:58:32.892 WARN release_tag_to_id_mapping:1567 Tag not found or entry is already clear.

Note that the first thing is does is unlock the mutex for the tag. That means that some other thread somewhere else created the tag and locked the mutex. That seems dangerous. Then the following errors indicate that the tag is already destroyed. The error from mutex_destroy shown seems to indicate that there is another thread with the mutex locked and thus the destruction of the mutex errors out.

At least from this perspective, it looks like there is a logical flow problem in the app code somewhere. Having one thread lock a mutex and a different thread unlock it means that you could easily run into situations where the first thread is still using the mutex or some third thread is waiting on it.

I will see if I can do a few tweaks to the code to try to find out more. I do not have good logging telling me which tag is being operated on and I would like to find that out here to see if my hypothesis is correct.

All that said, nothing you do should be able to cause a crash in the library. I will also look at tightening up some of the mutex handling on plc_tag_destroy to see if I can remove any possibility of multiple threads hitting that code at the same time.

from libplctag.

I see. I will review my code and my error handling with the mutexes. Thank you.

There are 3 PLC's in this scenario.

from libplctag.

I ran across a comment about the POSIX specification about mutexes.

According to Posix "If a thread attempts to unlock a mutex that it has not locked or a mutex which is unlocked, undefined behavior results."

See here: [http://www.realworldtech.com/forum/?threadid=168856&curpostid=168883](Real World Tech)

So, this may explain some of what you are seeing.

That said, clearly I just need to make the library work in a threadsafe manner. It does not make sense to have all potential users pay the price when the library could do it internally. It is not fun to debug though :-(

from libplctag.

That sounds good. I assumed the library was thread safe through the use of plc_tag_lock and plc_tag_unlock. Is there a way to check to see if a tag is locked before I try to unlock it? In my close_all_tags function I unlock all tags then destroy them, without checking if they're locked or not first.

from libplctag.

Oops, I missed your reply. Sorry about that!

Unlocking mutexes is another one of the wonderful things that POSIX does not help with. As the quote says, unlocking and unlocked mutex results in undefined behavior. Sigh.

In practical terms, I do not think I have run across an implementation that crashed. It usually returns an error I think.

The library is thread safe now. If your only use of the lock/unlock API calls was to guard against threads hitting the individual API calls, then you should be able to remove all of those extra calls.

from libplctag.

OK. Thank you. I use a scoped lock on the class functions now to protect against errors and crashes.

from libplctag.