Skip to content

Minor pthread cleanups. NFC#26856

Merged
sbc100 merged 6 commits into
emscripten-core:mainfrom
kleisauke:minor-pthread-cleanups
May 6, 2026
Merged

Minor pthread cleanups. NFC#26856
sbc100 merged 6 commits into
emscripten-core:mainfrom
kleisauke:minor-pthread-cleanups

Conversation

@kleisauke
Copy link
Copy Markdown
Collaborator

Split out from #13007.

kleisauke added 2 commits May 5, 2026 10:50
This is an automatic change generated by tools/maint/rebaseline_tests.py.

The following (1) test expectation files were updated by
running the tests with `--rebaseline`:

```
codesize/test_codesize_minimal_pthreads.json: 26370 => 26370 [+0 bytes / +0.00%]

Average change: +0.00% (+0.00% - +0.00%)
```
Comment thread system/lib/pthread/library_pthread.c Outdated
@sbc100
Copy link
Copy Markdown
Collaborator

sbc100 commented May 5, 2026

Thanks for all these recent changes!

Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

in pthread_create.c, should the comment read "any allocation we do here should not be considered leaks" ?

@kleisauke
Copy link
Copy Markdown
Collaborator Author

Good catch! Fixed with 3ea99f2.

Comment thread system/lib/pthread/pthread_create.c Outdated
void* restrict arg) {
// Note on LSAN: lsan intercepts/wraps calls to pthread_create so any
// allocation we do here should be considered leaks.
// allocation we do here should not be considered as leak.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The change from leaks -> as leak seems to make this less grammatically correct, doesn't it?

Maybe allocations should be plural?

allocations we do here should not be considered leaks ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

You're right, English isn't my first language. "considered as leak" sounded better to me, but it's actually incorrect.

Fixed with commit fee0b4f.

@sbc100 sbc100 merged commit 6085102 into emscripten-core:main May 6, 2026
27 of 30 checks passed
@kleisauke kleisauke deleted the minor-pthread-cleanups branch May 7, 2026 08:12
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