Defer early match autoload to mitigate client crashes

[norths7ar]

When confogl_match_autoload 1 is enabled, Confogl currently begins loading mode plugins from OnClientPutInServer, before the connecting client has completed sign-on. In the tested downstream setup, this repeatedly crashed the Windows client during its first loading screen when the server needed to load ZoneMod and restart the map as part of that connection. Reconnecting succeeded once that work had completed.

This change waits for a real client's player_connect_full event, then schedules the existing mode-loading path with RequestFrame. Before executing, it revalidates the client serial, autoload setting, and active-match state. A generation counter invalidates queued work when the map ends or a match unload occurs. Existing plugin loading, manual match commands, and map-restart behavior are otherwise unchanged.

Validation

• Client dumps consistently showed an access violation at shaderapidx9.dll + 0x612A.
• Immediate ZoneMod autoload reproduced the crash. Disabling autoload and loading the mode after joining succeeded.
• Delaying OnClientPutInServer by one frame still crashed, while a 1-second delay succeeded. In successful player_connect_full tests, the event occurred about 0.34-0.37 seconds after PutInServer.
• The crash also reproduced after unloading the mode and restoring the baseline plugins without restarting the server process, showing that it was tied to the autoload timing rather than initial server startup.
• The final upstream-based binary from commit 599d14ff passed both an initial ZoneMod autoload and a later autoload after the mode had unloaded while empty, without restarting the server process.
• The final source compiles with SourcePawn 1.12.0.7230, with only the existing CreateDialog deprecation warning, and git diff --check passes.

A pristine upstream server reproduction has not yet been completed. Subsequent client-side tracing identified an already-loading early return that skips disabling queued material rendering before another map load, leading to the observed null D3D buffer dereference. A targeted, temporary client-memory intervention succeeded twice, with withdrawal reproducing the original crash in the same client process. See the follow-up investigation and limitations. This PR remains a server-side timing workaround, not a fix to the client engine; the generation counter only guards against stale queued work.

This concerns Confogl's built-in confogl_match_autoload, not the separate confogl_autoloader plugin.

[A1mDev]

I’m not really convinced this should be considered a fix rather than a workaround.

The tests only demonstrate that moving autoload from OnClientPutInServer to player_connect_full makes the crash stop reproducing. The actual cause of the access violation hasn’t been identified.

The delay test is especially telling: deferring by one frame still crashes, while waiting roughly one second succeeds. That looks much more like a timing workaround — we’re simply avoiding the problematic window.

The generation counter also addresses a separate issue: preventing stale queued callbacks from running after a match unload or map end. It doesn’t address the cause of the crash itself.

I’d therefore describe this as a workaround for early autoload rather than a fix for the underlying crash. A proper fix would ideally reproduce this on a pristine upstream setup and identify what exactly about the autoload + map restart sequence causes the client-side AV in shaderapidx9.dll.

The workaround itself may be useful and worth keeping, but based on these results, I don’t think we can confidently claim that the underlying crash has been fixed.

[norths7ar]

I’m not really convinced this should be considered a fix rather than a workaround.

The tests only demonstrate that moving autoload from OnClientPutInServer to player_connect_full makes the crash stop reproducing. The actual cause of the access violation hasn’t been identified.

The delay test is especially telling: deferring by one frame still crashes, while waiting roughly one second succeeds. That looks much more like a timing workaround — we’re simply avoiding the problematic window.

The generation counter also addresses a separate issue: preventing stale queued callbacks from running after a match unload or map end. It doesn’t address the cause of the crash itself.

I’d therefore describe this as a workaround for early autoload rather than a fix for the underlying crash. A proper fix would ideally reproduce this on a pristine upstream setup and identify what exactly about the autoload + map restart sequence causes the client-side AV in shaderapidx9.dll.

The workaround itself may be useful and worth keeping, but based on these results, I don’t think we can confidently claim that the underlying crash has been fixed.

Thanks, that distinction is fair. I’m not claiming this fixes the underlying null-pointer dereference in the client renderer.
The scope of this PR is to avoid the early-autoload sequence that reproduced the crash in my tests. The one-second delay was only a diagnostic experiment; the proposed change uses player_connect_full as an explicit connection milestone before scheduling the existing load path. The generation counter only guards against stale queued work.
I’m happy to describe this as a workaround for the observed crash and add a source comment explaining the reason for the timing change. I don’t currently have enough evidence to identify the renderer’s internal failure.
Would you consider this timing change acceptable on that basis, or is a pristine upstream reproduction required before merging?

[norths7ar]

Following up with new client-side debugging results. The distinction still stands: this PR is a server-side workaround, but I now have much stronger evidence for the underlying client failure.

I reproduced the same shaderapidx9.dll+0x612A access violation using a minimal server-side probe that reloads the same map (c1m1_hotel) approximately 3 seconds after OnClientPutInServer, without running the Confogl mode-loading path. The client used cl_forcepreload 0 and mat_queue_mode -1; its actual material queue mode was traced separately.

The observed failure sequence is:

1. Client sign-on enables queued material rendering while the loading-screen flag is still set.
2. The next loading-start call sees that the client is already loading and returns early. This also skips the call that disables queued material rendering.
3. A later temporary disable scope restores the enabled state, and a loading-progress EndFrame applies queue mode 2 before displacement geometry is loaded.
4. The displacement mesh modification path reaches a vertex-buffer wrapper with CPU backing memory but a null D3D buffer pointer, producing the original null-pointer AV.

For this client build, the loading-start guard is at engine.dll+0xD3B43, its early-return branch at +0xD3B4A, and the skipped queue-disable call at +0xD3B71. These are build-specific offsets, not public API names.

I then tested a targeted, temporary client-memory intervention on an insecure client/server: call the existing queue-disable function once before that early return when the loading flag is set and actual queue mode is 2. It leaves the loading flag intact and does not rerun the loading-screen initialization or increase the server delay.

Test	Result
Intervention enabled	Second load completed; queued rendering resumed afterward
Intervention removed in the same client process	Same entry state and original AV reproduced
Intervention re-enabled in a new process using the same account	Second load completed again; queued rendering resumed afterward

This supports a client-engine correction to the rendering-state transition, but it is a debugger proof of concept, not a production patch. The tests have debugger timing overhead, the client still has addons, and a pristine upstream reproduction has not yet been completed.

These results support the workaround's rationale; they do not turn this PR into a client-engine fix. The PR changes when the existing autoload/restart sequence runs rather than eliminating the second map load. Also, the minimal 3-second failure occurred after player_connect_full, so I am not claiming that event alone is a universal safe boundary; the PR-specific successful tests remain the evidence for this particular timing change. The generation counter remains solely a guard against stale queued work.

[SirPlease]

This is a lot of AI jargon with likely a lot of AI assessment rather than personal findings.
Regardless, the workaround is viable.

In the future it'd be preferable if you were to mention the use of AI as well as to formulate a PR along with additional comments with your own wording.

[norths7ar]

That's fair. My knowledge stops at sourcemod, so I used ai to help dig into the engine side and ended up relying on it too much for the wording. I did the actual client testing myself and maybe should’ve made that clearer. I’ll keep it more in my own words next time.