Skip to content

Add glecuid#234

Open
ilhamft wants to merge 2 commits into
gleam-lang:mainfrom
ilhamft:main
Open

Add glecuid#234
ilhamft wants to merge 2 commits into
gleam-lang:mainfrom
ilhamft:main

Conversation

@ilhamft

@ilhamft ilhamft commented Oct 27, 2025

Copy link
Copy Markdown

No description provided.

@lpil

lpil commented Oct 29, 2025

Copy link
Copy Markdown
Member

Hello! Thank you for the addition!

I was reading the source and I noticed there's a large chunk of vendored code, but I couldn't identify how the users of this library would get alerted to security vulnerabilities for this code, and how they would get security patches for it, seeing as it has been isolated from the upstream source. How do you recommend they deal with this?

The counters lack sequential consistency on Erlang and thread safety on JavaScript, so you can get collisions with these numbers. Wouldn't this result in collisions with the generated ids?

@ilhamft

ilhamft commented Oct 29, 2025

Copy link
Copy Markdown
Author

Hi, thanks for reviewing the code.

For the vendored code is there an official way for including javascript dependency?

On Erlang counters perhaps I should use the atomics option instead of write_concurrency.
Can you clarify about the thread safety issue? Javascript runtime is single threaded so there shouldn't be any issue with that.

EDIT: I've addressed some of your concerns in the next branch. For Js dependency, I think better way to do it is to instruct the user to get it from npm instead of vendoring it.

@lpil

lpil commented Nov 20, 2025

Copy link
Copy Markdown
Member

For the vendored code is there an official way for including javascript dependency?

Having people run npm install dep is the way to go 👍

On Erlang counters perhaps I should use the atomics option instead of write_concurrency.

Atomics are always good if you can use them! They've much faster than all the other options.

Javascript runtime is single threaded so there shouldn't be any issue with that.

Each JS thread will have their own copy of the counter, so multiple threads can get the same value at the same time.

@ilhamft

ilhamft commented Dec 6, 2025

Copy link
Copy Markdown
Author

Vendored codes are removed on v2.0.0 and instruction to get the dependency is added to readme.

Each JS thread will have their own copy of the counter, so multiple threads can get the same value at the same time.

This won't cause collision, because each copy of the counter will be initialized with different starting value.

@lpil

lpil commented Dec 6, 2025

Copy link
Copy Markdown
Member

I just had a look and it's using the private API for constructing Gleam types. The public API is documented here: https://gleam.run/documentation/externals/

@ilhamft

ilhamft commented Dec 7, 2025

Copy link
Copy Markdown
Author

Fixed

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