Reproduction case for channel-safely crash

[IdahoEv]

The attached save reliably reproduces a crash related to channel-safely, in the current steam version of the game, at least on my machine. I will try to find another computer to test it on tomorrow and see if it happens there as well.

Reproduction instructions:

1. Load the attached save
2. 'enable channel-safely' in DFHack
3. unpause the game

For me, this reliably causes a crash back to the desktop within two minutes, often within a few seconds.

Version and System Info

DFHack 50.11-r5rc1
Dwarf Fortress (Steam) 50.11 (build 12327894)
MSI GL65 Leopard
Intel(R) Core(TM) i7-10750H CPU @ 2.60GHz 2.59 GHz
16GB RAM
Windows 11 Home Ver: 22H2 (build 22621.3007)

Save File Link

https://drive.google.com/file/d/1_K9vCo_-8fk1F9jLrFyBLzpcVVYj1IkM/view?usp=drive_link

[myk002]

stack trace:

#0  0x00000000012dec73 in ?? ()
#1  0x000000000184d2b5 in ?? ()
#2  0x00007ffff797d9ed in DFHack::Job::removeJob (job=job@entry=0x7fff90cc58f0)
    at /home/myk/src/dfhack/library/modules/Job.cpp:397
#3  0x00007ffff34dab40 in cancel_job (job=0x7fff90cc58f0)
    at /home/myk/src/dfhack/plugins/channel-safely/include/inlines.h:200

job canceling has been problematic for a while. We've also seen crashes when we try to remove DestroyBuilding jobs, which is why we had to remove that feature from gui/mass-remove. I'll see if we can address this. If not, we might have to remove channel-safely entirely for now

[cppcooper]

It looks like #3713 doesn't resolve the crash in that save file.

          Thread 2 "dwarfort" received signal SIGSEGV, Segmentation fault.
                                                                          [Switching to Thread 0x7fffec9ff6c0 (LWP 643563)]
0x0000000001555ec3 in ?? ()
(gdb) bt
#0  0x0000000001555ec3 in ?? ()
#1  0x0000000001c2bb35 in ?? ()
#2  0x00007fffd66df87d in DFHack::Job::removeJob (job=job@entry=0x7fff507d6cc0) at /home/jcooper/Documents/projects/dfhack/library/modules/Job.cpp:424
#3  0x00007fffd4ee011a in cancel_job (job=0x7fff507d6cc0) at /home/jcooper/Documents/projects/dfhack/plugins/channel-safely/include/inlines.h:200
#4  0x00007fffd4edd43e in cancel_job (map_pos=...) at /home/jcooper/Documents/projects/dfhack/plugins/channel-safely/include/inlines.h:230
#5  ChannelManager::manage_one (this=this@entry=0x7fffd4eef520 <ChannelManager::Get()::instance>, map_pos=..., set_marker_mode=set_marker_mode@entry=true, marker_mode=<optimized out>, marker_mode@entry=true)
    at /home/jcooper/Documents/projects/dfhack/plugins/channel-safely/channel-manager.cpp:186

It is clear there is a problem happening under removeJob, but what is a bit beyond my gdb skills. And the RE skills are non-existent.

I managed to get some info about the job after the crash:

{
  id: -469172310,
  list_link: 0x2c0564bff34de3c5,
  posting_index: -1,

  job_type: df::enums::job_type::DigChannel,
  job_subtype: df::enums::job_subtype_surgery::None,

  pos: {
    x: 51,
    y: 62,
    z: 166
  },

...removed data...
}

I spent some time debugging and found that before and after the crash took place the job id was a negative number. I don't know why that would have happened. It may be something my code has done or it could be something that always takes place. So far I've revised a number of functions to avoid handling a job if it is either:

A. negatively ID'd
B. or pointing to an invalid map position

After adding checks to prevent crashes based on invalid ID numbers I found it was crashing on accesses into map blocks. Looking at the job->pos indicated the map block was FARRRRR outside the bounds of the map. I've prevented all crashes inside channel-safely.. but now the crash takes place inside EventManager.

So basically, channel-safely is probably fubar'ing the job list. Will need to spend some time looking at top level processing for the plugin. Conditions for jobs to be looked at for example. From there it'll be a matter of noting every path to removeJob et al.

[cppcooper]

It was a silly design flaw. i.e. removeJob being called from a manageJobStartedEvent call stack which of course means the job list is mid iteration when a job is removed from the list.

I've made a fix. I'll be writing a better fix. Then I'll open a PR and link this.