Skip to content

Commit 75df1c0

Browse files
authored
Merge pull request #3713 from lethosor/fix-removejob-gcc-optimization
Hack to force GCC to invoke cancel_job() through DF's vtable
2 parents 8cffe3b + bb79755 commit 75df1c0

File tree

2 files changed

+27
-1
lines changed

2 files changed

+27
-1
lines changed

library/modules/Job.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,14 @@ bool DFHack::Job::removeJob(df::job* job) {
387387
// call the job cancel vmethod graciously provided by The Toady One.
388388
// job_handler::cancel_job calls job::~job, and then deletes job (this has
389389
// been confirmed by disassembly).
390-
world->jobs.cancel_job(job);
390+
391+
// HACK: GCC (starting around GCC 10 targeting C++20 as of v50.09) optimizes
392+
// out the vmethod call here regardless of optimization level, so we need to
393+
// invoke the vmethod manually through a pointer, as the Lua wrapper does.
394+
// `volatile` does not seem to be necessary but is included for good
395+
// measure.
396+
volatile auto cancel_job_method = &df::job_handler::cancel_job;
397+
(world->jobs.*cancel_job_method)(job);
391398

392399
return true;
393400
}

test/modules/job.lua

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
config.target = 'core'
2+
config.mode = 'title' -- alters world state, not safe when a world is loaded
3+
4+
function test.removeJob()
5+
-- removeJob() calls DF code, so ensure that that DF code is actually running
6+
7+
-- for an explanation of why this is necessary to check,
8+
-- see https://github.com/DFHack/dfhack/pull/3713 and Job.cpp:removeJob()
9+
10+
expect.nil_(df.global.world.jobs.list.next, 'job list is not empty')
11+
12+
local job = df.job:new() -- will be deleted by removeJob() if the test passes
13+
dfhack.job.linkIntoWorld(job)
14+
expect.true_(df.global.world.jobs.list.next, 'job list is empty')
15+
expect.eq(df.global.world.jobs.list.next.item, job, 'expected job not found in list')
16+
17+
expect.true_(dfhack.job.removeJob(job))
18+
expect.nil_(df.global.world.jobs.list.next, 'job list is not empty after removeJob()')
19+
end

0 commit comments

Comments
 (0)