TwinFan / LiveTraffic

LiveTraffic is an X-Plane multiplayer plugin, which fills your sky with live air traffic based on public flight tracking data.
https://twinfan.gitbook.io/livetraffic/
Other
99 stars 24 forks source link

Crashes reported after exception on create with empty posDeque #174

Closed TwinFan closed 4 years ago

TwinFan commented 4 years ago

Describe the bug Two users report crashes of X-Plane after seeing ASSERT FAILED: validForAcCreate(). While the exception should theoretically be handled correctly, the difference to #173 is that posDeque is completely empty. Such a plane should never have been considered for creation. Need to find how the queue could be emptied between the check before creation and actual creation! And why it then really crashes as in other cases (like #173) the exception is caught and handled properly.

To Reproduce Unfortunately yet unknown. Last endurance test of 2,5h at KLAX didn't come up with any problems.

Expected behavior At least no crash. Ideally not even try to create that said a/c.

Technical Info:

Log.txt One user sent in a complete log: Log Crash AC Create - Traffic Global intercepting.txt

Another so far only an excerpt, more requested:

LiveTraffic 1586999913.5 FATAL src\LTFlightData.cpp:1742/TryFetchNewPos: ASSERT FAILED: validForAcCreate()
a/c A1A2E0 WN1939 SimTime: 1586999913.5 - 2020-04-16 01:18:33
pAc == <null>
posDeque:
<empty>
posToAdd:
<empty>
LiveTraffic 1586999913.5 ERROR src\LTFlightData.cpp:2410/CreateAircraft: Exception occured while creating a/c A1A2E0 of type B737: LiveTraffic 1586999913.5 FATAL src\LTFlightData.cpp:1742/TryFetchNewPos: ASSERT FAILED: validForAcCreate()
LiveTraffic 1586999913.5 ERROR src\LTFlightData.cpp:2420/CreateAircraft: Could not create new object (memory?): A1A2E0
--=={This application has crashed!}==--

Also see Forum post

TwinFan commented 4 years ago

Received a full .rpt file, of which I could extract the .dmp file. This points to the calculation thread, specifically to line 969 of LTFlightData.cpp / LTFlightData::CalcNextPos:

        } else {
            // If there is no a/c yet then we need one past and
            // one or more future positions
            // If already the first pos is in the future then we aren't valid yet
 --===>     if (simTime < posDeque.front().ts())
                return false;

        }

From the failed ASSERT log entry we know that posDeque is completely empty. With an empty posDeque a call to posDeque.front() will return an invalid reference...then .ts() will just crash. This particular place can be easily guarded against a crash.

But that's a follow-up problem, not the root cause.

The root problem is that with an empty posDeque the plane should not even have started creation. Why the assert failed in the first place is not yet explained. Right before new LTAircraft there is another call to validForAcCreate(). By that time the call must have succeeded. From within LTAircraft's constructor TryFetchNewPos is called, which then ASSERTs validForAcCreate() and failed. Inbetween, something must have happened.

We need to look at the parallel processing. That should have been guarded by a lock...

TwinFan commented 4 years ago

Can't find any mistake in locking. Still yet unclear what could have caused the posDeque to become empty. Added more log output in case that exception occurs again.

Change CalcNextPos so that no crash should happen any longer at that position that crashed now.

Issue stays open until the root cause has been found.

TwinFan commented 4 years ago

User reports confirm: The crash is gone. Still need to figure out root cause.

TwinFan commented 4 years ago

Received new logs with ASSERT FAILED but also with the inserted previous state of posDeque:

LiveTraffic 1587412235.8 FATAL src\LTFlightData.cpp:1744/TryFetchNewPos: ASSERT FAILED: validForAcCreate()
a/c 4CACBA ABR1623 SimTime: 1587412235.8 - 2020-04-20 19:50:35
pAc == <null>
posDeque:
<empty>
posToAdd:
<empty>
LiveTraffic 1587412235.8 ERROR src\LTFlightData.cpp:2418/CreateAircraft: Exception occured while creating a/c 4CACBA of type B734: LiveTraffic 1587412235.8 FATAL src\LTFlightData.cpp:1744/TryFetchNewPos: ASSERT FAILED: validForAcCreate()

PosDeque before was:
1587412216.0: (48.32830, 2.81580) 32905.6ft  GND_OFF                           {h 140 , p   2, r   0} <h 140,  4545m @ 442kt,  -75ft/m>
1587412236.0: (48.29710, 2.85550) 32880.6ft  GND_OFF                           {h 140 , p   2, r   0}

Other occurences and this share a similarity: The second position's timestamp in posDeque (1587412236.0) is shortly before current sim time (1587412235.8). In LTAircraft::CalcPPos the following happens now during creating the LTAirctaft object:

  1. In a first regular call to fd.TryFetchNewPos we receive those two positions from the posDeque that we see in the above log sniplet, and they are removed from posDeque. posDeque is empty now. All good, as usual.
  2. A few lines later CalcPPos notices, that the end position is less than 0,5s away and calls fd.TryFetchNewPos a second time in the hope to get more current data. Usually, it is no problem calling fd.TryFetchNewPos when there is none. But we are still in object creation phase. That means that fd.pAc has not been set. And that makes fd.TryFetchNewPos believe that it needs to furnish the initial two positions (one in the past, one in the future). But posDeque is empty. And then the ASSERT fails as there are no position to give to a LTAircraft object in creation.

What is new about this, so that it occurs now only? During v1.50 I introduced that fd.TryFetchNewPos already removes the positions from posDeque. Previously that was done later in CalcNextPos only, but required for a lot of if...then's to deal with outdated positions in posDeque.

Fix is simply not to call fd.TryFetchNewPos while we are in creation. And as we are on it, we also don't call fd.TriggerCalcNewPos. (This had in turn caused the original crash in the position calculation thread, which also had no fd.pAc pointer and an empty posDeque.) We can do all that in the next cycle, 1/20s away, when fd.pAc is properly set.