Failing test on v16 and x86 platform #1290

Activity

1. JckXia commented on Mar 16, 2023

   

   JckXia

   on Mar 16, 2023

   Last edited by JckXia

   Member

   More actions

   I did some digging into this issue. So far, it appears that a normal CI run will reliably reproduce this error. Occasionally, they will succeed but that seems to be more of an exception than the norm.

   However, when I tried to run just this particular test using node16 on X86 (https://github.com/JckXia/node-addon-api/actions/runs/4440582410/jobs/7794559762), it will pass with flying colors each time. I thought async_progress_queue_worker was the confounding factor but that doesn't seem to make much of a difference.

   Does anyone have any other ideas that I could explore?

   Issues to persist when we are running on X86 only, but against all node versions: https://github.com/JckXia/node-addon-api/actions/runs/4440781888/jobs/7795016181
2. KevinEady commented on Mar 16, 2023

   

   KevinEady

   on Mar 16, 2023

   Contributor

   More actions

   Hi @JckXia ,

   Thanks for looking into this!

   Looking at test/index.js, the log of "Running test async_worker" should come after the the Promise for the async_progress_worker test resolves. Since it's not, I think it is still async_progress_worker hanging. There could be some random deadlocking / mutex stuff going on when running the full test suite vs a subset of tests that makes it seem like everything is right when in actuality things are... not.

   I remember awhile back we had a similar flakey test. It was due to the missing predicate condition for condition_variable.wait() calls: #1168 to fix this issue #1150 that has a very similar test timeout failure.

   I noticed we have some places in async_progress_worker that use condition variables with no predicate, eg:
   

   node-addon-api/test/async_progress_worker.cc

   Lines 38 to 42 in 0b53d88

   	for (int32_t idx = 0; idx < _times; idx++) {
   	data.progress = idx;
   	progress.Send(&data, 1);
   	_cv.wait(lock);
   	}

   Maybe this is the cause? If you can start with addressing this one and see if it fixes the hang. If so, we should look into changing the other places that use condition variables with no predicate:

   node-addon-api/test/threadsafe_function/threadsafe_function.cc

   Lines 92 to 94 in 0b53d88

   	while (!info->closeCalledFromJs) {
   	info->signal.wait(lk);
   	}

   node-addon-api/test/typed_threadsafe_function/typed_threadsafe_function.cc

   Lines 88 to 90 in 0b53d88

   	while (!info->closeCalledFromJs) {
   	info->signal.wait(lk);
   	}

   👀React with 👀1Reacted by Jack
3. JckXia commented on Mar 16, 2023

   

   JckXia

   on Mar 16, 2023

   Member

   More actions

   Hey @KevinEady! Thank you, I added a wait condition and it indeed seems to have resolved the issue.
   #1297
4. JckXia

   closed this as completedin #1297on Mar 17, 2023