[GH-ISSUE #65] A missing cuCtxSynchronize seems to be causing correctness issues or at least nondeterminism #21

Closed
opened 2026-10-26 22:23:40 +00:00 by bsakaguchi · 5 comments
Owner

Originally created by @person4268 on GitHub (Jun 22, 2026).
Original GitHub issue: https://github.com/Comfy-Org/comfy-aimdo/issues/65

As a prelude: Apologies for not having proper receipts or reproducers on most of this stuff, I didn't save my test generations and haven't spent my time validating what GLM 5.2 said here thoroughly (but the effect I'm noticing is real for me at least). I'll dump most of my reasoning below, but it does seem to be an comfy-aimdo issue, and the suggested change does seem to fix my issue, so I'll give it that. I don't have a strong enough GPU programming background to actually validate the change.

I am on ComfyUI commit 0d8b7510bdc5409f4a76c3191e122ddea50f4aa2 and comfy-aimdo version 0.4.10.
Hardware: Acer Predator PHN16-71, i7-13700HX, 16GB RAM, 4060 Laptop (8GB VRAM)

I noticed the following behavior:

  • When generating (using Zeta-Chroma BF16, derivative of Z-Image, so ~12GB), occasionally the preview samples would visibly corrupt and usually recover the next step, more than what normal CFG and sampling does. It didn't seem to be sampling related, and it'd even happen towards the end of sampling and irrecoverably damage the entire generation, rarely.
  • I used GLM 5.2 to instrument everything potentially involved and narrowed it down to, calling the model forward() twice with the same inputs would return different values, occasionally.
  • This also manifested as nondeterminism, the same seed would produce different images, though I do think it's moreso broken than just nondeterministic floating point stuff.
  • Using --disable-dynamic-vram seemed to fix the issue visually, the same generations would produce the same images. This very much suggests to me that it is comfy-aimdo related.
  • I had GLM look for potential issues or things to test, and it spammed a bunch of cuCtxSynchronize calls, and we bisected down to one singular added call that appeared to fix the issue.

That patch was as follows:

diff --git a/src/model-vbar.c b/src/model-vbar.c
index be84a22..3c49298 100644
--- a/src/model-vbar.c
+++ b/src/model-vbar.c
@@ -344,6 +344,8 @@ int vbar_fault(void *devctx, void *vbar, uint64_t offset, uint64_t size, uint32_
 
     set_devctx((AimdoContext *)devctx);
 
+    CHECK_CU(cuCtxSynchronize()); /* AIMDO_SYNC_HAMMER (fault-top) */
+
     size_t page_end = VBAR_GET_PAGE_NR_UP(offset + size);
 
     log(VVERBOSE, "%s (start): offset=%lldk, size=%lldk\n", __func__, (ull)(offset / K), (ull)(size / K));

It also wrote

Regression introduced in e3a06be (v0.2.3): vbar_fault gained a stopgap vbars_free(budget_deficit(0)) eviction that runs without first synchronizing against in-flight GPU work. Under VRAM pressure this can unmap pages still being read by a not-yet-retired compute kernel. The 0.4.10 double-sync commit added syncs to five other functions but missed this one. Pre-e3a06be versions are unaffected because vbar_fault did no eviction there.

if that's useful.

Originally created by @person4268 on GitHub (Jun 22, 2026). Original GitHub issue: https://github.com/Comfy-Org/comfy-aimdo/issues/65 As a prelude: Apologies for not having proper receipts or reproducers on most of this stuff, I didn't save my test generations and haven't spent my time validating what GLM 5.2 said here thoroughly (but the effect I'm noticing is real for me at least). I'll dump most of my reasoning below, but it does seem to be an comfy-aimdo issue, and the suggested change does seem to fix my issue, so I'll give it that. I don't have a strong enough GPU programming background to actually validate the change. I am on ComfyUI commit `0d8b7510bdc5409f4a76c3191e122ddea50f4aa2` and comfy-aimdo version `0.4.10`. Hardware: Acer Predator PHN16-71, i7-13700HX, 16GB RAM, 4060 Laptop (8GB VRAM) I noticed the following behavior: - When generating (using Zeta-Chroma BF16, derivative of Z-Image, so ~12GB), occasionally the preview samples would visibly corrupt and usually recover the next step, more than what normal CFG and sampling does. It didn't seem to be sampling related, and it'd even happen towards the end of sampling and irrecoverably damage the entire generation, rarely. - I used GLM 5.2 to instrument everything potentially involved and narrowed it down to, calling the model forward() twice with the same inputs would return different values, occasionally. - This also manifested as nondeterminism, the same seed would produce different images, though I do think it's moreso broken than just nondeterministic floating point stuff. - Using `--disable-dynamic-vram` seemed to fix the issue visually, the same generations would produce the same images. This very much suggests to me that it is comfy-aimdo related. - I had GLM look for potential issues or things to test, and it spammed a bunch of cuCtxSynchronize calls, and we bisected down to one singular added call that appeared to fix the issue. That patch was as follows: ```patch diff --git a/src/model-vbar.c b/src/model-vbar.c index be84a22..3c49298 100644 --- a/src/model-vbar.c +++ b/src/model-vbar.c @@ -344,6 +344,8 @@ int vbar_fault(void *devctx, void *vbar, uint64_t offset, uint64_t size, uint32_ set_devctx((AimdoContext *)devctx); + CHECK_CU(cuCtxSynchronize()); /* AIMDO_SYNC_HAMMER (fault-top) */ + size_t page_end = VBAR_GET_PAGE_NR_UP(offset + size); log(VVERBOSE, "%s (start): offset=%lldk, size=%lldk\n", __func__, (ull)(offset / K), (ull)(size / K)); ``` It also wrote > Regression introduced in e3a06be (v0.2.3): vbar_fault gained a stopgap vbars_free(budget_deficit(0)) eviction that runs without first synchronizing against in-flight GPU work. Under VRAM pressure this can unmap pages still being read by a not-yet-retired compute kernel. The 0.4.10 double-sync commit added syncs to five other functions but missed this one. Pre-e3a06be versions are unaffected because vbar_fault did no eviction there. if that's useful.
Author
Owner

@coderabbitai[bot] commented on GitHub (Jun 22, 2026):

🔗 Related PRs

Comfy-Org/comfy-aimdo#62 - Dev/double sync [merged]


🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!

<!-- gh-comment-id:4764569270 --> @coderabbitai[bot] commented on GitHub (Jun 22, 2026): <!-- This is an auto-generated issue plan by CodeRabbit --> <details> <summary>🔗 Related PRs</summary> Comfy-Org/comfy-aimdo#62 - Dev/double sync [merged] </details> --- <details> <summary> 🧪 Issue enrichment is currently in open beta.</summary> You can configure auto-planning by selecting labels in the issue_enrichment configuration. To disable automatic issue enrichment, add the following to your `.coderabbit.yaml`: ```yaml issue_enrichment: auto_enrich: enabled: false ``` </details> 💬 Have feedback or questions? Drop into our [discord](https://discord.gg/coderabbit)!
Author
Owner

@person4268 commented on GitHub (Jun 24, 2026):

I have samples now. All were done with a fixed seed of 1000.

Without patch, without `--disable-dynamic-vram` Image Image Image Image Image
Without patch, with `--disable-dynamic-vram` Image Image Image Image Image
With patch, without `--disable-dynamic-vram` Image Image Image Image Image
<!-- gh-comment-id:4792205483 --> @person4268 commented on GitHub (Jun 24, 2026): I have samples now. All were done with a fixed seed of 1000. <details> <summary>Without patch, without `--disable-dynamic-vram`</summary> <img width="1024" height="1024" alt="Image" src="https://github.com/user-attachments/assets/275473d2-4e01-4130-bf63-08fabb14ce37" /> <img width="1024" height="1024" alt="Image" src="https://github.com/user-attachments/assets/1a7b5ce6-1df4-4371-9cfe-f75cfbd8718d" /> <img width="1024" height="1024" alt="Image" src="https://github.com/user-attachments/assets/6ff7c7dc-ad27-4d60-be2e-4f38c1c3ec2d" /> <img width="1024" height="1024" alt="Image" src="https://github.com/user-attachments/assets/6a2a9b78-5f5f-4e48-bcd8-45debfb8b473" /> <img width="1024" height="1024" alt="Image" src="https://github.com/user-attachments/assets/0ca92f26-07e7-4891-b36f-b811b5cfcf0d" /> </details> <details> <summary>Without patch, with `--disable-dynamic-vram`</summary> <img width="1024" height="1024" alt="Image" src="https://github.com/user-attachments/assets/0cc12176-9aae-4006-b5b7-547088368617" /> <img width="1024" height="1024" alt="Image" src="https://github.com/user-attachments/assets/63fcff61-2ae4-450c-8ef9-1ee6ab5eb154" /> <img width="1024" height="1024" alt="Image" src="https://github.com/user-attachments/assets/0922ed5a-e503-4992-a2be-68d67a1d879a" /> <img width="1024" height="1024" alt="Image" src="https://github.com/user-attachments/assets/b35ae181-8256-4d2e-984f-3c2fcd92e665" /> <img width="1024" height="1024" alt="Image" src="https://github.com/user-attachments/assets/8536d6cb-383a-42d1-b807-94e36944988f" /> </details> <details> <summary>With patch, without `--disable-dynamic-vram`</summary> <img width="1024" height="1024" alt="Image" src="https://github.com/user-attachments/assets/122b64b3-71c6-4755-a38f-b6cea33a9c4d" /> <img width="1024" height="1024" alt="Image" src="https://github.com/user-attachments/assets/7fca5c65-0143-48a1-9797-83b48ff05879" /> <img width="1024" height="1024" alt="Image" src="https://github.com/user-attachments/assets/c977f931-97d3-46a1-b4bd-fa08222bbaa5" /> <img width="1024" height="1024" alt="Image" src="https://github.com/user-attachments/assets/65da9e23-5322-41e0-8079-fd348c4d04cc" /> <img width="1024" height="1024" alt="Image" src="https://github.com/user-attachments/assets/f4f91c4e-0784-48a2-bb68-107a56595abe" /> </details>
Author
Owner

@rattus128 commented on GitHub (Jun 29, 2026):

vbar_fault calling syncrhonize unconditionally is a total performance killer. vbars_free(budget_deficit(0)) will synchronize conditionally if it has something to do, so your bots conclusion looks inaccurate. Weights being stale or racing would be absolutely catastrophic to outputs so im not sure of the theory. Is there a lora involved (common for non-determinimism) ?

<!-- gh-comment-id:4828914220 --> @rattus128 commented on GitHub (Jun 29, 2026): vbar_fault calling syncrhonize unconditionally is a total performance killer. vbars_free(budget_deficit(0)) will synchronize conditionally if it has something to do, so your bots conclusion looks inaccurate. Weights being stale or racing would be absolutely catastrophic to outputs so im not sure of the theory. Is there a lora involved (common for non-determinimism) ?
Author
Owner

@person4268 commented on GitHub (Jun 29, 2026):

The images I posted should have workflows attached, and shouldn't have loras, quantization, sageattn, or anything like that involved. I tried to keep it as minimal as I could. No launch args other than --preview-size.

And yeah, I kinda did figure it was likely wrong, hence why I tried to downweight the theory.

But I mean, it does occasionally seem to mess up pretty badly, like in the 5th image. The pattern I was seeing in the preview was generation would be going fine, then there'd be one incredibly bad step ruining the image, and all subsequent steps weren't enough to fix it. If you're lucky and it happened early, you'd just get weird/bad composition.

<!-- gh-comment-id:4828995386 --> @person4268 commented on GitHub (Jun 29, 2026): The images I posted should have workflows attached, and shouldn't have loras, quantization, sageattn, or anything like that involved. I tried to keep it as minimal as I could. No launch args other than `--preview-size`. And yeah, I kinda did figure it was likely wrong, hence why I tried to downweight the theory. But I mean, it does occasionally seem to mess up pretty badly, like in the 5th image. The pattern I was seeing in the preview was generation would be going fine, then there'd be one incredibly bad step ruining the image, and all subsequent steps weren't enough to fix it. If you're lucky and it happened early, you'd just get weird/bad composition.
Author
Owner

@person4268 commented on GitHub (Sep 9, 2026):

Don't appear to be experiencing this anymore for whatever reason, whether that be recent changes or driver updates.

<!-- gh-comment-id:5603076571 --> @person4268 commented on GitHub (Sep 9, 2026): Don't appear to be experiencing this anymore for whatever reason, whether that be recent changes or driver updates.
Sign in to join this conversation.
No labels
pull-request
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Comfy-Org/comfy-aimdo#21
No description provided.