Skip to content

Commit 1dabb8c

Browse files
Retribution98Retribution98
authored
[JS] Fix is_generating check fails (openvinotoolkit#3490)
<!-- Keep your pull requests (PRs) as atomic as possible. That increases the likelihood that an individual PR won't be stuck because of adjacent problems, merge conflicts, or code review. Your merged PR is going to appear in the automatically generated release notes on GitHub. So the clearer the title the better. --> ## Description Reset `is_generating` flag before calling `report_error()` in the error path of `generatePerformInferenceThread` to prevent a race condition where the JS event loop could schedule the next `generate()` call before the native thread clears the flag. <!-- Jira ticket number (e.g., 123). Delete if there's no ticket. --> CVS-182798 ## Checklist: - [x] This PR follows [GenAI Contributing guidelines](https://github.com/openvinotoolkit/openvino.genai?tab=contributing-ov-file#contributing). <!-- Always follow them. If there are deviations, explain what and why. --> - [ ] Tests have been updated or added to cover the new code. <!-- Specify exactly which tests were added or updated. If the change isn't maintenance related, update the tests at https://github.com/openvinotoolkit/openvino.genai/tree/master/tests or explain in the description why the tests don't need an update. --> - [x] This PR fully addresses the ticket. <!--- If not, explain clearly what is covered and what is not. If follow-up pull requests are needed, specify in the description. --> - [ ] I have made corresponding changes to the documentation. <!-- Run github.com/\<username>/openvino.genai/actions/workflows/deploy_gh_pages.yml on your fork with your branch as a parameter to deploy a test version with the updated content. Replace this comment with the link to the built docs. If the documentation is updated in a separate PR, clearly specify it. --> --------- Signed-off-by: Kirill Suvorov <kirill.suvorov@intel.com>
1 parent a384cce commit 1dabb8c

File tree

12 files changed

+43
-34
lines changed

12 files changed

+43
-34
lines changed

‎src/js/include/llm_pipeline/init_worker.hpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#pragma once
22

3+
#include <atomic>
34
#include <napi.h>
45

56
#include "openvino/genai/llm_pipeline.hpp"
@@ -10,7 +11,7 @@ class InitWorker : public AsyncWorker {
1011
public:
1112
InitWorker(Function& callback,
1213
std::shared_ptr<ov::genai::LLMPipeline>& pipe,
13-
std::shared_ptr<bool> is_initializing,
14+
std::shared_ptr<std::atomic<bool>> is_initializing,
1415
const std::string model_path,
1516
std::string device,
1617
ov::AnyMap properties);
@@ -22,7 +23,7 @@ class InitWorker : public AsyncWorker {
2223

2324
private:
2425
std::shared_ptr<ov::genai::LLMPipeline>& pipe;
25-
std::shared_ptr<bool> is_initializing;
26+
std::shared_ptr<std::atomic<bool>> is_initializing;
2627
std::string model_path;
2728
std::string device;
2829
ov::AnyMap properties;

‎src/js/include/llm_pipeline/llm_pipeline_wrapper.hpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#pragma once
22

3+
#include <atomic>
34
#include <napi.h>
45

56
#include <thread>
@@ -22,6 +23,6 @@ class LLMPipelineWrapper : public Napi::ObjectWrap<LLMPipelineWrapper> {
2223

2324
private:
2425
std::shared_ptr<ov::genai::LLMPipeline> pipe = nullptr;
25-
std::shared_ptr<bool> is_initializing = std::make_shared<bool>(false);
26-
std::shared_ptr<bool> is_generating = std::make_shared<bool>(false);
26+
std::shared_ptr<std::atomic<bool>> is_initializing = std::make_shared<std::atomic<bool>>(false);
27+
std::shared_ptr<std::atomic<bool>> is_generating = std::make_shared<std::atomic<bool>>(false);
2728
};

‎src/js/include/vlm_pipeline/init_worker.hpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
#pragma once
55

6+
#include <atomic>
67
#include <napi.h>
78

89
#include "openvino/genai/visual_language/pipeline.hpp"
@@ -13,7 +14,7 @@ class VLMInitWorker : public AsyncWorker {
1314
public:
1415
VLMInitWorker(Function& callback,
1516
std::shared_ptr<ov::genai::VLMPipeline>& pipe,
16-
std::shared_ptr<bool> is_initializing,
17+
std::shared_ptr<std::atomic<bool>> is_initializing,
1718
const std::string model_path,
1819
std::string device,
1920
ov::AnyMap properties);
@@ -25,7 +26,7 @@ class VLMInitWorker : public AsyncWorker {
2526

2627
private:
2728
std::shared_ptr<ov::genai::VLMPipeline>& pipe;
28-
std::shared_ptr<bool> is_initializing;
29+
std::shared_ptr<std::atomic<bool>> is_initializing;
2930
std::string model_path;
3031
std::string device;
3132
ov::AnyMap properties;

‎src/js/include/vlm_pipeline/vlm_pipeline_wrapper.hpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
#pragma once
55

6+
#include <atomic>
67
#include <napi.h>
78

89
#include <thread>
@@ -26,6 +27,6 @@ class VLMPipelineWrapper : public Napi::ObjectWrap<VLMPipelineWrapper> {
2627

2728
private:
2829
std::shared_ptr<ov::genai::VLMPipeline> pipe = nullptr;
29-
std::shared_ptr<bool> is_initializing = std::make_shared<bool>(false);
30-
std::shared_ptr<bool> is_generating = std::make_shared<bool>(false);
30+
std::shared_ptr<std::atomic<bool>> is_initializing = std::make_shared<std::atomic<bool>>(false);
31+
std::shared_ptr<std::atomic<bool>> is_generating = std::make_shared<std::atomic<bool>>(false);
3132
};

‎src/js/include/whisper_pipeline/init_worker.hpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
#pragma once
55

6+
#include <atomic>
67
#include <napi.h>
78

89
#include "openvino/genai/whisper_pipeline.hpp"
@@ -11,7 +12,7 @@ class WhisperInitWorker : public Napi::AsyncWorker {
1112
public:
1213
WhisperInitWorker(Napi::Function& callback,
1314
std::shared_ptr<ov::genai::WhisperPipeline>& pipe,
14-
std::shared_ptr<bool> is_initializing,
15+
std::shared_ptr<std::atomic<bool>> is_initializing,
1516
std::string&& model_path,
1617
std::string&& device,
1718
ov::AnyMap&& properties);
@@ -22,7 +23,7 @@ class WhisperInitWorker : public Napi::AsyncWorker {
2223

2324
private:
2425
std::shared_ptr<ov::genai::WhisperPipeline>& pipe;
25-
std::shared_ptr<bool> is_initializing;
26+
std::shared_ptr<std::atomic<bool>> is_initializing;
2627
std::string model_path;
2728
std::string device;
2829
ov::AnyMap properties;

‎src/js/include/whisper_pipeline/pipeline_wrapper.hpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
#pragma once
55

6+
#include <atomic>
67
#include <napi.h>
78

89
#include "openvino/genai/whisper_pipeline.hpp"
@@ -19,6 +20,6 @@ class WhisperPipelineWrapper : public Napi::ObjectWrap<WhisperPipelineWrapper> {
1920

2021
private:
2122
std::shared_ptr<ov::genai::WhisperPipeline> pipe = nullptr;
22-
std::shared_ptr<bool> is_initializing = std::make_shared<bool>(false);
23-
std::shared_ptr<bool> is_generating = std::make_shared<bool>(false);
23+
std::shared_ptr<std::atomic<bool>> is_initializing = std::make_shared<std::atomic<bool>>(false);
24+
std::shared_ptr<std::atomic<bool>> is_generating = std::make_shared<std::atomic<bool>>(false);
2425
};

‎src/js/src/llm_pipeline/init_worker.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
InitWorker::InitWorker(Function& callback,
44
std::shared_ptr<ov::genai::LLMPipeline>& pipe,
5-
std::shared_ptr<bool> is_initializing,
5+
std::shared_ptr<std::atomic<bool>> is_initializing,
66
const std::string model_path,
77
const std::string device,
88
const ov::AnyMap properties)

‎src/js/src/llm_pipeline/llm_pipeline_wrapper.cpp‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
#include "include/tokenizer.hpp"
1212

1313
struct TsfnContext {
14-
TsfnContext(GenerateInputs inputs, std::shared_ptr<bool> is_generating)
14+
TsfnContext(GenerateInputs inputs, std::shared_ptr<std::atomic<bool>> is_generating)
1515
: inputs(inputs),
1616
is_generating(is_generating) {};
1717
~TsfnContext() {};
@@ -21,7 +21,7 @@ struct TsfnContext {
2121
std::optional<Napi::ThreadSafeFunction> streamer_tsfn;
2222

2323
GenerateInputs inputs;
24-
std::shared_ptr<bool> is_generating;
24+
std::shared_ptr<std::atomic<bool>> is_generating;
2525
std::shared_ptr<ov::genai::LLMPipeline> pipe = nullptr;
2626
std::shared_ptr<ov::AnyMap> generation_config = nullptr;
2727
std::shared_ptr<ov::AnyMap> options = nullptr;
@@ -33,7 +33,7 @@ void performInferenceThread(TsfnContext* context) {
3333
try {
3434
jsCallback.Call(
3535
{Napi::Error::New(env, "performInferenceThread error. " + message).Value(), env.Null()});
36-
} catch (std::exception& err) {
36+
} catch (const std::exception& err) {
3737
std::cerr << "The callback failed when attempting to return an error from performInferenceThread. "
3838
"Details:\n"
3939
<< err.what() << std::endl;
@@ -73,7 +73,7 @@ void performInferenceThread(TsfnContext* context) {
7373
} else {
7474
resultPromise.set_value(ov::genai::StreamingStatus::RUNNING);
7575
}
76-
} catch (std::exception& err) {
76+
} catch (const std::exception& err) {
7777
streamer_exceptions.push_back(err.what());
7878
resultPromise.set_value(ov::genai::StreamingStatus::CANCEL);
7979
}
@@ -100,8 +100,11 @@ void performInferenceThread(TsfnContext* context) {
100100
}},
101101
context->inputs);
102102

103-
} catch (std::exception& e) {
103+
} catch (const std::exception& e) {
104+
*context->is_generating = false;
104105
report_error(e.what());
106+
finalize();
107+
return;
105108
}
106109
// should be called right after inference to release the flag asap
107110
*context->is_generating = false;
@@ -124,7 +127,7 @@ void performInferenceThread(TsfnContext* context) {
124127
env.Null(), // Error should be null in normal case
125128
to_decoded_result(env, result) // Return DecodedResults as the final result
126129
});
127-
} catch (std::exception& err) {
130+
} catch (const std::exception& err) {
128131
report_error("The final callback failed. Details:\n" + std::string(err.what()));
129132
}
130133
});
@@ -133,7 +136,7 @@ void performInferenceThread(TsfnContext* context) {
133136
report_error("The final BlockingCall failed with status " + status);
134137
}
135138
}
136-
} catch (std::exception& e) {
139+
} catch (const std::exception& e) {
137140
report_error(e.what());
138141
}
139142
finalize();

‎src/js/src/vlm_pipeline/init_worker.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
VLMInitWorker::VLMInitWorker(Function& callback,
77
std::shared_ptr<ov::genai::VLMPipeline>& pipe,
8-
std::shared_ptr<bool> is_initializing,
8+
std::shared_ptr<std::atomic<bool>> is_initializing,
99
const std::string model_path,
1010
const std::string device,
1111
const ov::AnyMap properties)

‎src/js/src/vlm_pipeline/vlm_pipeline_wrapper.cpp‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
#include "include/vlm_pipeline/start_chat_worker.hpp"
1515

1616
struct VLMTsfnContext {
17-
VLMTsfnContext(VLMGenerateInputs inputs, std::shared_ptr<bool> is_generating)
17+
VLMTsfnContext(VLMGenerateInputs inputs, std::shared_ptr<std::atomic<bool>> is_generating)
1818
: inputs(std::move(inputs)),
1919
is_generating(is_generating) {};
2020
~VLMTsfnContext() {};
@@ -26,7 +26,7 @@ struct VLMTsfnContext {
2626
VLMGenerateInputs inputs;
2727
std::vector<ov::Tensor> images;
2828
std::vector<ov::Tensor> videos;
29-
std::shared_ptr<bool> is_generating;
29+
std::shared_ptr<std::atomic<bool>> is_generating;
3030
std::shared_ptr<ov::genai::VLMPipeline> pipe = nullptr;
3131
std::shared_ptr<ov::AnyMap> generation_config = nullptr;
3232
};
@@ -37,7 +37,7 @@ void vlmPerformInferenceThread(VLMTsfnContext* context) {
3737
try {
3838
jsCallback.Call(
3939
{Napi::Error::New(env, "vlmPerformInferenceThread error. " + message).Value(), env.Null()});
40-
} catch (std::exception& err) {
40+
} catch (const std::exception& err) {
4141
std::cerr << "The callback failed when attempting to return an error from vlmPerformInferenceThread. "
4242
"Details:\n"
4343
<< err.what() << std::endl;
@@ -76,7 +76,7 @@ void vlmPerformInferenceThread(VLMTsfnContext* context) {
7676
} else {
7777
resultPromise.set_value(ov::genai::StreamingStatus::RUNNING);
7878
}
79-
} catch (std::exception& err) {
79+
} catch (const std::exception& err) {
8080
streamer_exceptions.push_back(err.what());
8181
resultPromise.set_value(ov::genai::StreamingStatus::CANCEL);
8282
}
@@ -102,9 +102,9 @@ void vlmPerformInferenceThread(VLMTsfnContext* context) {
102102
}},
103103
context->inputs);
104104

105-
} catch (std::exception& e) {
106-
report_error(e.what());
105+
} catch (const std::exception& e) {
107106
*context->is_generating = false;
107+
report_error(e.what());
108108
finalize();
109109
return;
110110
}
@@ -127,7 +127,7 @@ void vlmPerformInferenceThread(VLMTsfnContext* context) {
127127
env.Null(),
128128
to_vlm_decoded_result(env, result),
129129
});
130-
} catch (std::exception& err) {
130+
} catch (const std::exception& err) {
131131
report_error("The final callback failed. Details:\n" + std::string(err.what()));
132132
}
133133
});
@@ -136,7 +136,7 @@ void vlmPerformInferenceThread(VLMTsfnContext* context) {
136136
report_error("The final BlockingCall failed with status " + status);
137137
}
138138
}
139-
} catch (std::exception& e) {
139+
} catch (const std::exception& e) {
140140
report_error(e.what());
141141
}
142142
finalize();
@@ -223,8 +223,8 @@ Napi::Value VLMPipelineWrapper::generate(const Napi::CallbackInfo& info) {
223223
}
224224
context->native_thread = std::thread(vlmPerformInferenceThread, context);
225225
} catch (const std::exception& ex) {
226-
Napi::Error::New(env, ex.what()).ThrowAsJavaScriptException();
227226
*this->is_generating = false;
227+
Napi::Error::New(env, ex.what()).ThrowAsJavaScriptException();
228228
}
229229
return env.Undefined();
230230
}

0 commit comments

Comments
 (0)