FS#3130 - economy.cpp:1110: void LoadUnloadVehicle(Vehicle*, int*): Assertion `v->load_unload_time_rem != 0'

Attached to Project: OpenTTD
Opened by fonsinchen (fonsinchen) - Monday, 17 August 2009, 20:03 GMT
Last edited by Remko Bijker (Rubidium) - Wednesday, 19 August 2009, 15:28 GMT
Type Bug
Category Core
Status Closed
Assigned To No-one
Operating System All
Severity Low
Priority Normal
Reported Version Version?
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


This assertion can be triggered in rare cases when a vehicle is reverted in a station while loading. Unfortunately I can't tell the exact condition, but with a slight modification of the code and a savegame I'll prove that it can actually happen.

Consider this piece of code in LoadUnloadVehicle:

if (v->type == VEH_TRAIN && (!IsTileType(v->tile, MP_STATION) || GetStationIndex(v->tile) != st->index)) {
/* The train reversed in the station. Take the "easy" way
* out and let the train just leave as it always did. */
SetBit(v->vehicle_flags, VF_LOADING_FINISHED);

Obviously here LoadUnloadVehicle is left with v->current_order.type at OT_LOADING and v->load_unload_time_rem == 0. This is bad as we have seen in  FS#3054 . Yet when leaving this way the order gets changed in the next call to HandleLoading. However there are cases when LoadUnloadVehicle is called again before the order gets changed. In order to trigger the effect more often the condition is commented out. This has the same effect as if every vehicle was always entering a too short station and was immediately reversed by the player. In fact this would theoretically be possible. Then load the attached save game. It is rather large and has a lot of vehicles which further increases the chance of the bad code path being executed. You'll quickly get the following assertion:

(gdb) bt
#0 0x00007f2317e96ed5 in raise () from /lib/
#1 0x00007f2317e983f3 in abort () from /lib/
#2 0x00007f2317e8fdc9 in __assert_fail () from /lib/
#3 0x00000000004fa0e1 in LoadUnloadVehicle (v=0x3046890, cargo_left=0x7fff22a97970) at /home/alve/projekte/openttd/src/economy.cpp:1110
#4 0x00000000004fb586 in LoadUnloadStation (st=0x31a5d10) at /home/alve/projekte/openttd/src/economy.cpp:1393
#5 0x00000000006a0e0f in CallVehicleTicks () at /home/alve/projekte/openttd/src/vehicle.cpp:599
#6 0x00000000005b4296 in StateGameLoop () at /home/alve/projekte/openttd/src/openttd.cpp:1159
#7 0x00000000005b562f in GameLoop () at /home/alve/projekte/openttd/src/openttd.cpp:1242
#8 0x00000000006ae5a7 in VideoDriver_SDL::MainLoop (this=0x29372e0) at /home/alve/projekte/openttd/src/video/sdl_v.cpp:501
#9 0x00000000005b640e in ttd_main (argc=1, argv=0x7fff22a98148) at /home/alve/projekte/openttd/src/openttd.cpp:733
#10 0x0000000000699476 in main (argc=1, argv=0x7fff22a98148) at /home/alve/projekte/openttd/src/unix.cpp:251

This proves that there is a way to get into LoadUnloadVehicle after leaving the same method at that place and before changing the current order. So I suggest to either track down that code path (which I couldn't) or set load_unload_time_rem to 1, like it is done a few lines further down for a similar problem.
This task depends upon

Closed by  Remko Bijker (Rubidium)
Wednesday, 19 August 2009, 15:28 GMT
Reason for closing:  Fixed
Additional comments about closing:  In r17222
Comment by fonsinchen (fonsinchen) - Monday, 17 August 2009, 20:12 GMT
This is in trunk, of course, r17206. And this is probably the same as  FS#3129 . I just didn't see that one in time. In fact I was trying to reproduce the problem in, but couldn't with the supplied save game.