OverviewFilesCommitsIssuesDocsReleasesPeers

nvidia-rtx-godot commits / 229d005

RD: fix swapped index/validator decode, and name the double-freed resource
The invalid-free classifier (4bd871b1f8) decoded the RID backwards. Its comment claimed "RID_Alloc packs (index << 32) | validator" and it printed index as the high word. rid_owner.h is unambiguous the other way: :157 builds the id as `validator = 1 + (_gen_id() % 0x7FFFFFFF); id = validator; id <<= 32;`, :215/:319 /:347 all read `validator = uint32_t(id >> 32)`, and :382 reconstructs ids as `(validator << 32) | i` where i is the element index. So every line this thing has printed was mislabelled, and in a way that inverts the conclusion: `_gen_id()` is `base_id.increment()`, a process-wide monotonic counter, so the big climbing numbers reported as "index" are validators (allocation birth order) and the small ones reported as "validator" are slot indices. Read correctly, the hyper-tps logs show small recurring indices with rising validators -- which is exactly the slot-reuse signature the classifier was written to detect, while its output said the opposite. Also records which owner an ID belonged to when it was legitimately freed, and reports that on the double free. "was acceleration_structure" points at one cache; an unnamed RID points at seventeen. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
229d005c2ef9be60d7fb894ff43588349155e023
by plankatron <fleeky@gmail.com> · 2026-07-31T18:36:07.000Z

Changed files (2) +63 -13

servers/rendering/rendering_device.cpp +56 -7
……@@ -7733,6 +7733,46 @@ void RenderingDevice::_free_internal(RID p_id) {
77337733 // record which IDs were genuinely freed. That history is what lets a later
77347734 // invalid free be reported as a DOUBLE free rather than just "invalid".
77357735 bool dbg_was_invalid = false;
7736
7737 // Which owner held this ID, captured BEFORE the chain below frees it. Knowing
7738 // a double-freed ID was an acceleration structure rather than a uniform set is
7739 // the difference between one suspect cache and seventeen.
7740 const char *dbg_owner_kind = nullptr;
7741 if (texture_owner.owns(p_id)) {
7742 dbg_owner_kind = "texture";
7743 } else if (framebuffer_owner.owns(p_id)) {
7744 dbg_owner_kind = "framebuffer";
7745 } else if (sampler_owner.owns(p_id)) {
7746 dbg_owner_kind = "sampler";
7747 } else if (vertex_buffer_owner.owns(p_id)) {
7748 dbg_owner_kind = "vertex_buffer";
7749 } else if (vertex_array_owner.owns(p_id)) {
7750 dbg_owner_kind = "vertex_array";
7751 } else if (index_buffer_owner.owns(p_id)) {
7752 dbg_owner_kind = "index_buffer";
7753 } else if (index_array_owner.owns(p_id)) {
7754 dbg_owner_kind = "index_array";
7755 } else if (shader_owner.owns(p_id)) {
7756 dbg_owner_kind = "shader";
7757 } else if (uniform_buffer_owner.owns(p_id)) {
7758 dbg_owner_kind = "uniform_buffer";
7759 } else if (texture_buffer_owner.owns(p_id)) {
7760 dbg_owner_kind = "texture_buffer";
7761 } else if (storage_buffer_owner.owns(p_id)) {
7762 dbg_owner_kind = "storage_buffer";
7763 } else if (uniform_set_owner.owns(p_id)) {
7764 dbg_owner_kind = "uniform_set";
7765 } else if (render_pipeline_owner.owns(p_id)) {
7766 dbg_owner_kind = "render_pipeline";
7767 } else if (compute_pipeline_owner.owns(p_id)) {
7768 dbg_owner_kind = "compute_pipeline";
7769 } else if (acceleration_structure_owner.owns(p_id)) {
7770 dbg_owner_kind = "acceleration_structure";
7771 } else if (raytracing_pipeline_owner.owns(p_id)) {
7772 dbg_owner_kind = "raytracing_pipeline";
7773 } else if (hit_sbt_owner.owns(p_id)) {
7774 dbg_owner_kind = "hit_sbt";
7775 }
77367776#endif
77377777#ifdef DEV_ENABLED
77387778 String resource_name;
……@@ -7894,14 +7934,23 @@ void RenderingDevice::_free_internal(RID p_id) {
78947934 // "never valid" -> it was never a live RID here: uninitialised or
78957935 // corrupted memory, or an ID from another RD instance.
78967936 //
7897 // The RID is also decoded: RID_Alloc packs (index << 32) | validator, so a
7898 // recurring index with a changing validator is slot reuse -- the signature
7899 // of stale handles surviving a free/realloc cycle.
7937 // The RID is also decoded. RID_Alloc packs (validator << 32) | index --
7938 // see rid_owner.h:157-158 (`id = validator; id <<= 32;`) and :382
7939 // (`_make_from_id((validator << 32) | i)`). The validator is a process-wide
7940 // monotonic counter (`_gen_id()` -> `base_id.increment()`), so it reads as
7941 // allocation birth-order; the index is the slot in this owner. A recurring
7942 // index with a rising validator is slot reuse -- the signature of a stale
7943 // handle surviving a free/realloc cycle.
79007944 String dbg_decoded;
79017945#ifdef DEBUG_ENABLED
7902 String dbg_kind = _dbg_freed_ids.has(p_id.get_id()) ? "DOUBLE FREE (freed before)" : "never valid";
7903 dbg_decoded = " [index=" + itos(p_id.get_id() >> 32) +
7904 " validator=" + itos(p_id.get_id() & 0xFFFFFFFF) + " " + dbg_kind + "]";
7946 String dbg_kind = "never valid";
7947 if (_dbg_freed_ids.has(p_id.get_id())) {
7948 dbg_kind = "DOUBLE FREE (freed before, was ";
7949 dbg_kind += _dbg_freed_ids[p_id.get_id()];
7950 dbg_kind += ")";
7951 }
7952 dbg_decoded = " [validator=" + itos(p_id.get_id() >> 32) +
7953 " index=" + itos(p_id.get_id() & 0xFFFFFFFF) + " " + dbg_kind + "]";
79057954#endif
79067955
79077956 if (invalid_free_count <= PRINT_LIMIT) {
……@@ -7921,7 +7970,7 @@ void RenderingDevice::_free_internal(RID p_id) {
79217970
79227971#ifdef DEBUG_ENABLED
79237972 if (!dbg_was_invalid) {
7924 _dbg_freed_ids.insert(p_id.get_id());
7973 _dbg_freed_ids.insert(p_id.get_id(), dbg_owner_kind ? dbg_owner_kind : "unknown");
79257974 }
79267975#endif
79277976
servers/rendering/rendering_device.h +7 -6
……@@ -1913,12 +1913,13 @@ protected:
19131913public:
19141914 void _free_internal(RID p_id);
19151915#ifdef DEBUG_ENABLED
1916 // Every RID this device has successfully freed. Used only to classify an
1917 // invalid free as a DOUBLE free (stale handle held past the resource's life)
1918 // versus an ID that was never valid here (uninitialised/corrupt). Debug-only:
1919 // it grows with the number of freed resources and is a diagnostic, not a
1920 // correctness mechanism.
1921 HashSet<uint64_t> _dbg_freed_ids;
1916 // Every RID this device has successfully freed, mapped to the owner it came
1917 // from. Used only to classify an invalid free as a DOUBLE free (stale handle
1918 // held past the resource's life) versus an ID that was never valid here
1919 // (uninitialised/corrupt), and to say WHAT the stale handle referred to.
1920 // Debug-only: it grows with the number of freed resources and is a
1921 // diagnostic, not a correctness mechanism.
1922 HashMap<uint64_t, const char *> _dbg_freed_ids;
19221923#endif
19231924 void _begin_frame(bool p_presented = false);
19241925 void _end_frame();