Skip to content

Comments

Hydrate file thread safe#188

Merged
dajimenezriv-internxt merged 11 commits intomasterfrom
hydrate-file-thread-safe
Sep 15, 2025
Merged

Hydrate file thread safe#188
dajimenezriv-internxt merged 11 commits intomasterfrom
hydrate-file-thread-safe

Conversation

@dajimenezriv-internxt
Copy link
Contributor

No description provided.

@dajimenezriv-internxt dajimenezriv-internxt self-assigned this Sep 8, 2025
Base automatically changed from extract-hydrate-file to master September 15, 2025 15:03
static std::string GetFileIdentity(const wchar_t *path);
static void HydrateFile(const wchar_t *filePath);
static void DehydrateFile(const wchar_t *filePath);
static void DeleteFileSyncRoot(const wchar_t *path);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not used in javascript so we can remove it.

}

#define NAPI_SAFE_WRAP(env, info, fn) \
napi_safe_wrap(env, info, fn, __FUNCTION__)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically we have created this wrapper so we log also the name of the function that is wrapping. Why? This is wrapper that wraps all unexpected C++ exceptions and converts them in a javascript exception (napi_throw_error). We concatenate the message with the name of the function FUNCTION.


if (SUCCEEDED(hr)) {
connectionMap[syncRootPath] = *connectionKey;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we have removed the try catch since we already obtaining this using the previous NAPI_SAFE_WRAP in Wrappers.cpp

size_t pathLength;
napi_get_value_string_utf16(env, argv[0], nullptr, 0, &pathLength);
syncRootPath = new WCHAR[pathLength + 1];
napi_get_value_string_utf16(env, argv[0], reinterpret_cast<char16_t *>(const_cast<wchar_t *>(syncRootPath)), pathLength + 1, nullptr);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could lead to a memory leak.

{
napi_throw_error(env, nullptr, "cancelFetchDataCallback should be a function.");
return nullptr;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we control the input from javascript we don't need to validate here that we pass functions, otherwise it will break in development and we make this function clearer and simpler.

napi_create_promise(env, &deferred, &promise);

// Lanzar la operación asíncrona en un hilo separado
std::thread([deferred, fullPath, env]()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was leading to a crash in the sync process when we were hydrating a file already hydrated. Why? This is a native thread of C++, not one from the napi (javascript). Now we create an async work using napi.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The try catchs have been encapsulated into execute_work function and complete_work.

@dajimenezriv-internxt dajimenezriv-internxt merged commit dc3f154 into master Sep 15, 2025
1 check passed
@dajimenezriv-internxt dajimenezriv-internxt deleted the hydrate-file-thread-safe branch September 15, 2025 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants