Crash uploading mission items in gRPC #46
Assignees
Labels
Comments
|
According to https://groups.google.com/forum/#!topic/grpc-io/M84p6FD-BcE
|
|
I am not sure if it is a crash in the upload. From the logs, it seems like it may come from the mission progress stream (which is, IMO, more likely to create a |
|
@julianoes: Did you manage to reproduce it? Would be nice to understand where that happens (e.g. in upload, start, or mission progress). |
|
Yes, trying to reproduce now. |
|
Ok, it happened: And now all threads: |
|
@JonasVautherin could this be a leak given we have multiple threads subscribed to mission progress and also multiple for the in-air status? |
|
So it happens in
Looking at the code, I don't think we leak. If we register multiple times, the old callbacks will never be called so the stream will be hanging there until the caller cancels the call, but that's not exactly a leak. gRPC supports having multiple calls on the same function. What could happen, maybe, is that two threads could be trying to Could that be that they aren't called on the same thread on our C++ side? |
Right I noticed that it only happens in the case where updates are fired at (almost) the same time.
Do they need to be called on the same thread? They might be on the thread pool in which case it might be different threads from the pool. |
In which case we need a mutex on the shared My question would then be: does it make sense to call them on different threads (as in: "couldn't it happen that the order is not preserved?")? And of course, maybe it is not so easy to make sure they are called on the same thread, which is possibly why we added the mutex in the camera. |
|
Some more info:
|
|
The question is why we have two reports in short succession, that's also odd. |
|
What's your point there? 😅
Yes. And what does -1 mean? Is it related to @zulufoxtrot's first point: "The only occurence of crashes is right after mission upload."? Like... I don't know, when a new mission overwrites an older one, then somehow -1 is emitted and directly 0 after that? When playing with the python examples, I was also feeling like sometimes I receive the 0 event, and sometimes the first one is 1. @zulufoxtrot: Thanks for those points, that's super constructive 👍! Very good bug report :-). |
|
New full output, crash at second upload: |
|
@JonasVautherin right, so I guess we should fix both, the double output with |
|
@julianoes thanks, you guys are doing all the hard work :) Now I just got a crash at first upload. However the output is different: |
|
Interesting, this time it's not the |
Wrong order here 😅. That's generally not an issue, but we should keep that in mind.
Or sometimes it ends up with a segv. I could imagine that, too.
@julianoes: What about you take the |
|
I'm already trying the mutex but now only one mission update arrives on the Python side which is odd. |
|
@JonasVautherin I wish I could, but I'm non proficient in C++. |
No worries, reporting and testing is already super helpful! Thanks a lot for that 👍. |
|
Yet another one: |
|
Fix coming up in: mavlink/MAVSDK#668 |
|
@JonasVautherin should we release |
|
@zulufoxtrot: would you mind testing your code against the latest |
|
Sure, I'll try and report back soon. |
|
Been running for 30 minutes, about 30 uploads, no crash so far. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
A crash report by that needs to be investigated:
https://paste.ofcode.org/qmzXxgYH2LkVpYiyXBGJ5u
The text was updated successfully, but these errors were encountered: