-
Notifications
You must be signed in to change notification settings - Fork 22
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
pvput segfaults for bo over ca #114
Comments
This works for me with PVA (you can run 'softIocPVA' to test), so I think this is a caProvider bug. @mrkraimer is this a case which fixed by #96. As a general request. When reporting a crash, if at all possible please try reproduce with a debug build (eg. 'linux-x86_64-debug') as this will yield better information. |
valgrind reports this as a use after free
|
Why is this assigned to me? |
@mdavidsaver I can confirm that it works when using |
I poked around a bit more. Some context first: I discovered this issue while using So I was allocating a Here's how I was using it originally, for context: class PutCallback : public ClientChannel::PutCallback
{
PVStructure::const_shared_pointer const & value;
BitSet const & tosend;
epicsEvent done;
Operation operation;
public:
PutCallback(pvac::ClientChannel& channel,
PVStructure::const_shared_pointer const & request,
PVStructure::const_shared_pointer const & value,
BitSet const & bitSet) :
value(value), tosend(bitSet),
operation(channel.put(this, request)) // put() starts here
{}
virtual void putBuild(const StructureConstPtr &build,
ClientChannel::PutCallback::Args& args) OVERRIDE FINAL
{
args.root = value;
args.tosend = tosend;
}
virtual void putDone(const pvac::PutEvent &evt) OVERRIDE FINAL
{
switch(evt.event) {
case pvac::PutEvent::Fail:
fprintf(stderr, "PutCallback Error: %s\n", evt.message.c_str());
break;
case pvac::PutEvent::Cancel:
fprintf(stderr, "PutCallback Canceled\n");
break;
case pvac::PutEvent::Success:
break;
}
done.signal();
}
void wait(double timeout = 0.0)
{
if (timeout)
done.wait(timeout);
else
done.wait();
}
}; And the call site: void Subscription::put(PVStructurePtr const & value, BitSet const & bitSet)
{
if (mConnected) {
// leaks, doesn't SEGFAULT
PutCallback *pc = new PutCallback(mChannel, mRequest, value, bitSet);
// doesn't leak, SEGFAULTs
//PutCallback pc(mChannel, mRequest, value, bitSet);
pc->wait();
}
} |
@brunoseivam The existing code is known to have a number of problems. Can you test Marty's #96 ? This is a large change, so imo. there isn't much point to spending time with the existing code. |
I'll do that tomorrow and report back |
Tests with @mrkraimer's fork. Merge
Test
Maybe automatic merging didn't work very well? Using @mrkraimer's fork as is
TestingI am not going to paste the results here because they are really similar to the previous ones. Sometimes I'd be happy to run any more tests that you guys would want me to. |
I suspect that the problem is related to put callbacks being called from a ca thread. In https://github.com/mrkraimer/pva2ca I added code so that all put and get client callbacks are called from a non ca thread. I ran extensive tests without any failures. But not so easy for bruno to test because he is basing his code on pvAccessCPP/src/client/pva/client.h If, however, Michael will let me move the code from mrkraimer/pva2ca I do not want to spend time doing this unless I will also be allowed to merge this code into |
Bruno, I can then clone it and see if I can reproduce what You see. |
Hi Marty, I'll try to do that later on today, but in the mean time the issue can be reproduced with From Marty I am using the latest epics-base. what version of base are you using? |
Hi Marty, Sorry for the late reply. My system:
I just freshly redownloaded and recompiled everything like so:
On a terminal:
On another terminal:
Then I updated all submodules inside
Recompiled
Let me know if you want me to test anything else. |
Replicated, with back-trace:
Then elsewhere:
That was with the pvAccess master branch version of pvput. Using the master branch from
Thus I believe this problem should be fixed by Marty's work in #96 although the above shows that there are still some debugging messages that need removing. @mdavidsaver agreed to the merge at a Hangout that we have minutes for, although it hasn't happened yet; @mrkraimer thought I was supposed to hit the Merge button, but I would rather the maintainer do that himself. We're going to be talking about this at the Hangout on Tuesday whether we want to or not... |
I wonder why @mrkraimer's repo fails on my setup then. Here's with the backtrace:
|
NOTE I did the following with pvAccessCPP from both mrkraimer and from epics-base. I started:
Note that
Then in another window:
And
I have no idea why it fails for Bruno and succeeds for me. Does anyone have any idea what is happening? Bruno what do you get when you
Lets make sure we have the same version of base |
|
One difference on the server-side is that I was running |
Here are the SHAs of the PVA submodules I was running, which are newer than the latest commit to 7.0:
For pvAccess that last commit was this:
To update all the submodules to their latest commits, run the
This version of pvget still segfaults, Marty's does not:
|
Note that pvAccess hash differs because it is Marty's repo, all the other ones match with Andrew's.
I still see segfaults. |
I think I know what is causing the crash.
This is NOT a fix but I want to see if I am on the correct path. Marty |
You seem to be on the right track there Marty:
So |
Far fewer crashes when specifying |
Marty, I just tried this on your branch; with
Don't know if this has anything to do with the original crash, but it's obviously a problem that needs fixing. BTW I talked to Michael, once you have fixed these smaller issues I will merge your changes. |
I also get the same
I am now convinced that block=false is the problem.
But if block=false then ca calls the client put callback as soon as the put request is issued. I am going to fix this first in be the same. |
I have pushed changes to |
Hi Marty, Your changes seem to have worked here, thanks! However, on another note, when I issue a PutCallback the I will put together some code that triggers it. I can open a separate issue if that's more appropriate. As Andrew suggest in the next message please open a separate issue. |
Thanks Marty, this commit seems to have solved all the problems I was seeing. @brunoseivam please report your other problem as a separate issue (was that specific to the CA provider? That error message appears in the generic code, so it isn't clear). |
I created a new issue. See #115. |
Released with Base 7.0.2 |
Maybe it is related to #93 ?
On another terminal:
The text was updated successfully, but these errors were encountered: