fix spritesheet gen on linux

[Kashargul]

The suffering with spritesheet generation and iconforge in three acts.

Act1: The DM side mishandling in a subsystem

Quite soon after porting and implementing icon forge, the first errors showed up:
[2025-08-08T18:17:20] Runtime in code/modules/asset_cache/iconforge/batched_spritesheet.dm,200: Spritesheet robot_icons_miner UNKNOWN ERROR: NO SUCH JOB

At that time, little was known what a box of pandora this error would open up, but let's go through.

https://discord.com/channels/484170914754330625/485190298050363393/1403446760504033281

While discussing the issue on coderbus, sophie gave the right hint about assets being invoked multiple times. Let's break down what happened..

code\controllers\subsystems\asset_loading.dm

/// Allows us to lazyload asset datums
/// Anything inserted here will fully load if directly gotten
/// So this just serves to remove the requirement to load assets fully during init
SUBSYSTEM_DEF(asset_loading)
	name = "Asset Loading"
	priority = FIRE_PRIORITY_ASSETS
	flags = SS_NO_INIT
	runlevels = RUNLEVEL_LOBBY|RUNLEVELS_DEFAULT
	var/list/datum/asset/generate_queue = list()
	var/last_queue_len = 0

/datum/controller/subsystem/asset_loading/fire(resumed)
    while(length(generate_queue))
        var/datum/asset/to_load = generate_queue[generate_queue.len]

        to_load.queued_generation()

        if(MC_TICK_CHECK)
            return

        last_queue_len = length(generate_queue)
        generate_queue.len--
    // We just emptied the queue
    if(last_queue_len && !length(generate_queue))
        // Clean up cached icons, freeing memory.
        rustg_iconforge_cleanup()

/datum/controller/subsystem/asset_loading/proc/queue_asset(datum/asset/queue)
#ifdef DO_NOT_DEFER_ASSETS
	stack_trace("We queued an instance of [queue.type] for lateloading despite not allowing it")
#endif
	generate_queue += queue

/datum/controller/subsystem/asset_loading/proc/dequeue_asset(datum/asset/queue)
	generate_queue -= queue

code\modules\asset_cache\iconforge\batched_spritesheet.dm

/datum/asset/spritesheet_batched/queued_generation()
	realize_spritesheets(yield = TRUE)


/datum/asset/spritesheet_batched/proc/realize_spritesheets(yield)
	if(fully_generated)
		return
	if(!length(entries))
		CRASH("Spritesheet [name] ([type]) is empty! What are you doing?")

	if(isnull(entries_json))
		entries_json = json_encode(entries)

	if(isnull(cache_result))
		cache_result = should_refresh(yield)
		if(cache_result == CACHE_WAIT) // sleep interrupted by MC. We'll get queried again later.
			cache_result = null
			return

	// read_from_cache returns false if config is disabled, otherwise it fully loads the spritesheet.
	if (cache_result == CACHE_VALID && read_from_cache())
		SSasset_loading.dequeue_asset(src)
		fully_generated = TRUE
		return
	// Remove the cache, since it's invalid if we get to this point.
	fdel("[ASSET_CROSS_ROUND_SMART_CACHE_DIRECTORY]/spritesheet_cache.[name].json")

	var/do_cache = CONFIG_GET(flag/smart_cache_assets) || force_cache
	var/data_out
	if(yield || !isnull(job_id))
		if(isnull(job_id))
			job_id = rustg_iconforge_generate_async("data/spritesheets/", name, entries_json, do_cache, FALSE, TRUE)
		UNTIL((data_out = rustg_iconforge_check(job_id)) != RUSTG_JOB_NO_RESULTS_YET)
		SSasset_loading.assets_generating--
	else
		data_out = rustg_iconforge_generate("data/spritesheets/", name, entries_json, do_cache, FALSE, TRUE)
	if (data_out == RUSTG_JOB_ERROR)
		CRASH("Spritesheet [name] JOB PANIC")
	else if(!findtext(data_out, "{", 1, 2))
		rustg_file_write(entries_json, "[GLOB.log_directory]/spritesheet_debug_[name].json")
		CRASH("Spritesheet [name] UNKNOWN ERROR: [data_out]")
	var/data = json_decode(data_out)
	sizes = data["sizes"]
	sprites = data["sprites"]
	var/input_hash = data["sprites_hash"]
	var/dmi_hashes = data["dmi_hashes"] // this only contains values if do_cache is TRUE.

	for(var/size_id in sizes)
		var/png_name = "[name]_[size_id].png"
		var/file_directory = "data/spritesheets/[png_name]"
		var/file_hash = rustg_hash_file(RUSTG_HASH_MD5, file_directory)
		SSassets.transport.register_asset(png_name, fcopy_rsc(file_directory), file_hash)
		if(CONFIG_GET(flag/save_spritesheets))
			save_to_logs(file_name = png_name, file_location = file_directory)
	var/css_name = "spritesheet_[name].css"
	var/file_directory = "data/spritesheets/[css_name]"

	fdel(file_directory)
	var/css = generate_css()
	rustg_file_write(css, file_directory)
	var/css_hash = rustg_hash_string(RUSTG_HASH_MD5, css)
	SSassets.transport.register_asset(css_name, fcopy_rsc(file_directory), file_hash=css_hash)

	if(CONFIG_GET(flag/save_spritesheets))
		save_to_logs(file_name = css_name, file_location = file_directory)

	if (do_cache)
		write_cache_meta(input_hash, dmi_hashes)
	fully_generated = TRUE
	// If we were ever in there, remove ourselves
	SSasset_loading.dequeue_asset(src)
	if(data["error"] && !(ignore_dir_errors && findtext(data["error"], "is not in the set of valid dirs")))
		CRASH("Error during spritesheet generation for [name]: [data["error"]]")

If we look at the original code, we were calling the asset generation in sync, our UNTIL proc now slept the subsystem while the asset was generated. This was our first "race condition". How, well we had a subsystem that was sleeping and could be woken again by the MC. This led to the same asset being called multiple times. Not to mention the wrong tick check, which was also placed quite badly... it alone could lead to double executions in case we ever hit a TICK_CHECK and jumped out.

The fix and what followed:
code\controllers\subsystems\asset_loading.dm

SUBSYSTEM_DEF(asset_loading)
	name = "Asset Loading"
	priority = FIRE_PRIORITY_ASSETS
	flags = SS_NO_INIT
	runlevels = RUNLEVEL_LOBBY|RUNLEVELS_DEFAULT
	var/list/datum/asset/generate_queue = list()
	var/assets_generating = 0
	var/last_queue_len = 0

/datum/controller/subsystem/asset_loading/fire(resumed)
	while(length(generate_queue))
		var/datum/asset/to_load = generate_queue[generate_queue.len]


		last_queue_len = length(generate_queue)
		generate_queue.len--

		to_load.queued_generation()

		if(MC_TICK_CHECK)
			return

	// We just emptied the queue
	if(last_queue_len && !length(generate_queue) && !assets_generating)
		// Clean up cached icons, freeing memory.
		rustg_iconforge_cleanup()

/datum/controller/subsystem/asset_loading/proc/queue_asset(datum/asset/queue)
#ifdef DO_NOT_DEFER_ASSETS
	stack_trace("We queued an instance of [queue.type] for lateloading despite not allowing it")
#endif
	generate_queue += queue

/datum/controller/subsystem/asset_loading/proc/dequeue_asset(datum/asset/queue)
	generate_queue -= queue

In the subsystem, a counter was added to count the currently externally generated assets, so that we won't clean up too early.

/datum/asset/spritesheet_batched/queued_generation()
	INVOKE_ASYNC(src, PROC_REF(realize_spritesheets), TRUE) // The proc is called inside a subsystem and waits with an UNTIL


/datum/asset/spritesheet_batched/proc/realize_spritesheets(yield)
	if(fully_generated)
		return
	if(!length(entries))
		CRASH("Spritesheet [name] ([type]) is empty! What are you doing?")

	if(getting_genned)
		stack_trace("Spritesheet batching has been called twice. This is illegal!")
	if(isnull(entries_json))
		entries_json = json_encode(entries)

	if(isnull(cache_result))
		cache_result = should_refresh(yield)
		if(cache_result == CACHE_WAIT) // sleep interrupted by MC. We'll get queried again later.
			cache_result = null
			return

	// read_from_cache returns false if config is disabled, otherwise it fully loads the spritesheet.
	if (cache_result == CACHE_VALID && read_from_cache())
		SSasset_loading.dequeue_asset(src)
		fully_generated = TRUE
		return
	// Remove the cache, since it's invalid if we get to this point.
	fdel("[ASSET_CROSS_ROUND_SMART_CACHE_DIRECTORY]/spritesheet_cache.[name].json")

	var/do_cache = CONFIG_GET(flag/smart_cache_assets) || force_cache
	var/data_out
	if(yield || !isnull(job_id))
		if(isnull(job_id))
			getting_genned = TRUE
			SSasset_loading.assets_generating++
			job_id = rustg_iconforge_generate_async("data/spritesheets/", name, entries_json, do_cache, FALSE, TRUE)
		UNTIL((data_out = rustg_iconforge_check(job_id)) != RUSTG_JOB_NO_RESULTS_YET)
		getting_genned = FALSE
		SSasset_loading.assets_generating--
	else
		data_out = rustg_iconforge_generate("data/spritesheets/", name, entries_json, do_cache, FALSE, TRUE)
	if (data_out == RUSTG_JOB_ERROR)
		CRASH("Spritesheet [name] JOB PANIC")
	else if(!findtext(data_out, "{", 1, 2))
		rustg_file_write(entries_json, "[GLOB.log_directory]/spritesheet_debug_[name].json")
		CRASH("Spritesheet [name] UNKNOWN ERROR: [data_out]")
	var/data = json_decode(data_out)
	sizes = data["sizes"]
	sprites = data["sprites"]
	var/input_hash = data["sprites_hash"]
	var/dmi_hashes = data["dmi_hashes"] // this only contains values if do_cache is TRUE.

	for(var/size_id in sizes)
		var/png_name = "[name]_[size_id].png"
		var/file_directory = "data/spritesheets/[png_name]"
		var/file_hash = rustg_hash_file(RUSTG_HASH_MD5, file_directory)
		SSassets.transport.register_asset(png_name, fcopy_rsc(file_directory), file_hash)
		if(CONFIG_GET(flag/save_spritesheets))
			save_to_logs(file_name = png_name, file_location = file_directory)
	var/css_name = "spritesheet_[name].css"
	var/file_directory = "data/spritesheets/[css_name]"

	fdel(file_directory)
	var/css = generate_css()
	rustg_file_write(css, file_directory)
	var/css_hash = rustg_hash_string(RUSTG_HASH_MD5, css)
	SSassets.transport.register_asset(css_name, fcopy_rsc(file_directory), file_hash=css_hash)

	if(CONFIG_GET(flag/save_spritesheets))
		save_to_logs(file_name = css_name, file_location = file_directory)

	if (do_cache)
		write_cache_meta(input_hash, dmi_hashes)
	fully_generated = TRUE
	// If we were ever in there, remove ourselves
	SSasset_loading.dequeue_asset(src)
	if(data["error"] && !(ignore_dir_errors && findtext(data["error"], "is not in the set of valid dirs")))
		CRASH("Error during spritesheet generation for [name]: [data["error"]]")

Our actual spritesheet generation was invoked asynchronously to our subsystem to prevent bad duplicated calls. If someone wants to test this on the old code, just add the getting_genned variable as implemented above, the old subsystem will cause those crashes if a codebase has many or large assets.

The box of pandora... We now had the cases that we were firing rust-g jobs faster to rust-g than they would finish. Rust-g works with threads, this led to parallel execution of the asset generation.

Why does this only show on linux. In short, linux handles read/write accesses as well as CWD differently than windows. Or one could say windows hosters were just lucky this time.

Act2: parallel spritesheet generation

Let's look into what happened.
Runtime in code/modules/asset_cache/iconforge/batched_spritesheet.dm, line 239: Error during spritesheet generation for vending: Failed to open DMI 'icons/obj/lighters.dmi' - No such file or directory (os error 2) Failed to open DMI 'icons/obj/ecig.dmi' - No such file or directory (os error 2) Failed to open DMI 'icons/obj/ecig.dmi' - No such file or directory (os error 2) Failed to open DMI 'icons/obj/ecig.dmi' - No such file or directory (os error 2) Failed to open DMI 'icons/obj/ecig.dmi' - No such file or directory (os error 2) Failed to open DMI 'icons/obj/ecig.dmi' - No such file or directory (os error 2) Failed to open DMI 'icons/obj/ecig.dmi' - No such file or directory (os error 2) Failed to open DMI 'icons/obj/chaplain.dmi' - No such file or directory (os error 2) Failed to open DMI 'icons/obj/ecig.dmi' - No such file or directory (os error 2) Failed to open DMI 'icons/inventory/suit/item.dmi' - No such file or directory (os error 2) Failed to open DMI 'icons/obj/ecig.dmi' - No such file or directory (os error 2) Failed to open DMI 'icons/inventory/suit/item.dmi' - No such file or directory (os error 2) Failed to open DMI 'icons/obj/ecig.dmi' - No such file or directory (os error 2) Failed to open DMI 'icons/obj/weapons.dmi' - No such file or directory (os error 2) Failed to open DMI 'icons/obj/ecig.dmi' - No such file or directory (os error 2) Failed to open DMI 'icons/obj/food.dmi' - No such file or directory (os error 2) Failed to open DMI 'icons/obj/ecig.dmi' - No such file or directory (os error 2) Failed to open DMI 'icons/obj/lighters.dmi' - No such file or directory (os error 2) Failed to open DMI 'icons/pdrink.dmi' - No such file or directory (os error 2) Failed to open DMI 'icons/pdrink.dmi' - No such file or directory (os error 2) Failed to open DMI 'icons/pdrink.dmi' - No such file or directory (os error 2) Failed to open DMI 'icons/obj/food.dmi' - No such file or directory (os error 2) Failed to open DMI 'icons/obj/chaplain.dmi' - No such file or directory (os error 2) Image was requested but does not exist in the cache. It's likely that the icon state doesn't exist: UniversalIcon(icon_file=icons/obj/chaplain.dmi, icon_state=mrobe, dir=Some(2), frame=None) - while generating 'godfig' No such file or directory (os error 2)

Our filepath to dmi function is neither thread safe not directory safe as it relies on cwd. More about the cwd issues further down. This can cause errors on file accesses in heavy parallel workloads.

/// Given a DMI filepath, returns a DMI Icon structure and caches it.
pub fn filepath_to_dmi(icon_path: &str) -> Result<Arc<Icon>, String> {
    zone!("filepath_to_dmi");
    {
        zone!("check_dmi_exists");
        if let Some(found) = ICON_FILES.get(icon_path) {
            return Ok(found.clone());
        }
    }
    let icon_file = match File::open(icon_path) {
        Ok(icon_file) => icon_file,
        Err(err) => {
            return Err(format!("Failed to open DMI '{icon_path}' - {err}"));
        }
    };
    let reader = BufReader::new(icon_file);
    let dmi: Icon;
    {
        zone!("parse_dmi");
        dmi = match Icon::load(reader) {
            Ok(dmi) => dmi,
            Err(err) => {
                return Err(format!("DMI '{icon_path}' failed to parse - {err}"));
            }
        };
    }
    {
        zone!("insert_dmi");
        let dmi_arc = Arc::new(dmi);
        let other_arc = dmi_arc.clone();
        // Cache it for later, saving future DMI parsing operations, which are very slow.
        ICON_FILES.insert(icon_path.to_owned(), dmi_arc);
        Ok(other_arc)
    }
}

A simple test which fails quite reliably, especially for file access, we don't want parallel access to the same file.

#[cfg(test)]
mod tests {
    use super::filepath_to_dmi;
    use std::{sync::Arc, thread};
    use rand::seq::SliceRandom;
    use rand::rng;

    #[test]
    fn stress_test_dmi_cache_reads_and_writes() {
        let icon_paths = vec![
            "tests/dm/rsc/iconforge_gags_dm.dmi",
            "tests/dm/rsc/iconforge_tests.dmi",
        ];

        let mut handles = Vec::new();

        for _ in 0..16 {
            let paths = icon_paths.clone();
            handles.push(thread::spawn(move || {
                let mut rng = rng();
                let mut paths = paths;
                for _ in 0..500 {
                    paths.shuffle(&mut rng);
                    let path = &paths[0];
                    let result = filepath_to_dmi(path);
                    assert!(result.is_ok(), "Failed to load DMI {path:?}");
                    let first_arc: Arc<_> = result.unwrap();

                    let second_arc = filepath_to_dmi(path).unwrap();
                    assert!(Arc::ptr_eq(&first_arc, &second_arc), "Cache returned different Arc for {path:?}");
                }
            }));
        }

        for h in handles {
            h.join().expect("Thread panicked during cache stress test");
        }

        println!("Concurrent read/write stress test passed!");
    }
}

Multiple sheets getting genned always could call the same icon, they then called the same, global cache which can lead to a race condition. This entire function needs to be able to handle parallel access.

I'm also not sure why there were locks over entire, large parallel workloads... the locks should be as local as possible to utilize the parallel computation properly...

Anyway, let's go on, this later down the line led to the following error. Which always was present, just so rare that it often slipped compared to the above one.

Act3: current working directory on linux

Error during write:
Runtime in code/modules/asset_cache/iconforge/batched_spritesheet.dm, line 239: Error during spritesheet generation for robot_icons_gravekeeper: Failed to save PNG file 'data/spritesheets/robot_icons_gravekeeper_120x120.png': No such file or directory (os error 2)

What we have here, is a common case of a CWD issue. Icon job operations all depend on the current working directory. This can change under linux at any time. So our assumption of the path might no longer be the same once execution finishes and our write fails. This is something that likely should be changed for all of those jobs. The path should be pinned once. It's static in our use case anyway.

Some more comments later in the PR code itself on each section.

[itsmeow]

Very thorough and helpful! This is the kind of thing I was lacking experience in when it comes to parallelism and multi threading, so thank you for taking the time to work this out.

[itsmeow]

Can you post a patch file for the DM changes, since the API is codebase dependent?

[Kashargul]

Can you post a patch file for the DM changes, since the API is codebase dependent?

dmside_changes.patch

diff --git a/code/controllers/subsystem/asset_loading.dm b/code/controllers/subsystem/asset_loading.dm
index 30c3078a0d9..d8139caf148 100644
--- a/code/controllers/subsystem/asset_loading.dm
+++ b/code/controllers/subsystem/asset_loading.dm
@@ -7,6 +7,7 @@ SUBSYSTEM_DEF(asset_loading)
 	flags = SS_NO_INIT | SS_HIBERNATE
 	runlevels = RUNLEVEL_LOBBY|RUNLEVELS_DEFAULT
 	var/list/datum/asset/generate_queue = list()
+	var/assets_generating = 0
 	var/last_queue_len = 0
 
 /datum/controller/subsystem/asset_loading/PreInit()
@@ -19,14 +20,17 @@ SUBSYSTEM_DEF(asset_loading)
 	while(length(generate_queue))
 		var/datum/asset/to_load = generate_queue[generate_queue.len]
 
+
+		last_queue_len = length(generate_queue)
+		generate_queue.len--
+
 		to_load.queued_generation()
 
 		if(MC_TICK_CHECK)
 			return
-		last_queue_len = length(generate_queue)
-		generate_queue.len--
+
 	// We just emptied the queue
-	if(last_queue_len && !length(generate_queue))
+	if(last_queue_len && !length(generate_queue) && !assets_generating)
 		// Clean up cached icons, freeing memory.
 		rustg_iconforge_cleanup()
 
diff --git a/code/modules/asset_cache/spritesheet/batched/batched_spritesheet.dm b/code/modules/asset_cache/spritesheet/batched/batched_spritesheet.dm
index a5a636327f5..23c116536af 100644
--- a/code/modules/asset_cache/spritesheet/batched/batched_spritesheet.dm
+++ b/code/modules/asset_cache/spritesheet/batched/batched_spritesheet.dm
@@ -46,6 +46,7 @@
 	var/cache_dmi_hashes_json = null
 	/// Used to prevent async cache refresh jobs from looping on failure.
 	var/cache_result = null
+	var/getting_genned = FALSE
 
 /datum/asset/spritesheet_batched/proc/should_load_immediately()
 #ifdef DO_NOT_DEFER_ASSETS
@@ -168,6 +169,9 @@
 	if(!length(entries))
 		CRASH("Spritesheet [name] ([type]) is empty! What are you doing?")
 
+	if(getting_genned)
+		stack_trace("Spritesheet batching has been called twice. This is illegal!")
+
 	if(isnull(entries_json))
 		entries_json = json_encode(entries)
 
@@ -189,8 +193,12 @@
 	var/data_out
 	if(yield || !isnull(job_id))
 		if(isnull(job_id))
+			getting_genned = TRUE
+			SSasset_loading.assets_generating++
 			job_id = rustg_iconforge_generate_async("data/spritesheets/", name, entries_json, do_cache, FALSE, TRUE)
 		UNTIL((data_out = rustg_iconforge_check(job_id)) != RUSTG_JOB_NO_RESULTS_YET)
+		getting_genned = FALSE
+		SSasset_loading.assets_generating--
 	else
 		data_out = rustg_iconforge_generate("data/spritesheets/", name, entries_json, do_cache, FALSE, TRUE)
 	if (data_out == RUSTG_JOB_ERROR)
@@ -232,7 +240,7 @@
 		CRASH("Error during spritesheet generation for [name]: [data["error"]]")
 
 /datum/asset/spritesheet_batched/queued_generation()
-	realize_spritesheets(yield = TRUE)
+	INVOKE_ASYNC(src, PROC_REF(realize_spritesheets), TRUE) // The proc is called inside a subsystem and waits with an UNTIL
 
 /datum/asset/spritesheet_batched/ensure_ready()
 	if(!fully_generated)

[ZeWaka]

Looks all correct to me.

Talked to Kash and this is all backwards-compatible, will include DM patch snippet on what to fix in the release notes, with reminders in the next major as well.