fix: use std::shared_ptr instead of std::unique_ptr for shared ressource, use operator[] instead of find_field by lemire · Pull Request #11 · anonrig/node · GitHub
Skip to content

fix: use std::shared_ptr instead of std::unique_ptr for shared ressource, use operator[] instead of find_field#11

Merged
lemire merged 1 commit into
anonrig:rewrite-task-runner-in-cppfrom
lemire:rewrite-task-runner-in-cpp
Apr 24, 2024
Merged

fix: use std::shared_ptr instead of std::unique_ptr for shared ressource, use operator[] instead of find_field#11
lemire merged 1 commit into
anonrig:rewrite-task-runner-in-cppfrom
lemire:rewrite-task-runner-in-cpp

Conversation

@lemire

@lemire lemire commented Apr 24, 2024

Copy link
Copy Markdown
Collaborator
  • In simdjson, we can use find_field or find_field_unordered. The operator[] is mapped to find_field_unordered. The find_field is fine, but it is used when you know the order of the keys, which is more appropriate for private JSON uses.
  • I feel uneasy about having one std:: unique_ptr that gets passed as a pointer. To me, it suggests that std:: shared_ptr is more appropriate.
  • The reset call on the object is fine.
  • I have avoided some temporary std::string instances although it is probably not significant.

@lemire lemire merged commit 92dacf0 into anonrig:rewrite-task-runner-in-cpp Apr 24, 2024
anonrig pushed a commit that referenced this pull request Jun 11, 2026
This function call can fail with `Z_VERSION_ERROR` if the compiled
library vs loaded library mismatched in version number or in
stream structure size.
In those cases, zlib doesn't initialize the `strm_.msg` field to
null. Therefore, when a `CompressionError` object is created via
`ErrorForMessage()`, it can read a stale or uninitialized `strm_.msg`
pointer that will cause a crash.

Example ASAN report:
```
AddressSanitizer: SEGV on unknown address
    #0 __strlen_avx2
        string/../sysdeps/x86_64/multiarch/strlen-avx2.S:76
    #1 strlen (/work/node/out/Debug/node+0x1a42ab7)
    #2 v8::(anonymous namespace)::StringLength(char const*)
        /work/node/out/../deps/v8/src/api/api.cc:7581:16
    #3 v8::(anonymous namespace)::StringLength(unsigned char const*)
        /work/node/out/../deps/v8/src/api/api.cc:7587:10
    #4 v8::String::NewFromOneByte(v8::Isolate*,
        unsigned char const*, v8::NewStringType, int)
        /work/node/out/../deps/v8/src/api/api.cc:7677:3
    #5 node::OneByteString(v8::Isolate*,
        char const*, int, v8::NewStringType)
        /work/node/out/../src/util-inl.h:166:10
    #6 node::(anonymous namespace)::CompressionStream<
        node::(anonymous namespace)::ZlibContext>
        ::EmitError(node::(anonymous namespace)
        ::CompressionError const&)
        /work/node/out/../src/node_zlib.cc:565:7
    #7 node::(anonymous namespace)::CompressionStream<
        node::(anonymous namespace)::ZlibContext>
        ::CheckError()
        /work/node/out/../src/node_zlib.cc:519:5
    #8 node::(anonymous namespace)::CompressionStream<
        node::(anonymous namespace)::ZlibContext>
        ::AfterThreadPoolWork(int)
        /work/node/out/../src/node_zlib.cc:543:10
    #9 node::ThreadPoolWork::ScheduleWork()
        ::'lambda'(uv_work_s*, int)
        ::operator()(uv_work_s*, int) const
        /work/node/out/../src/threadpoolwork-inl.h:57:15
    #10 node::ThreadPoolWork::ScheduleWork()
        ::'lambda'(uv_work_s*, int)
        ::__invoke(uv_work_s*, int)
        /work/node/out/../src/threadpoolwork-inl.h:48:7
    #11 uv__work_done /work/libuv-1.51.0/src/threadpool.c:330:5
    #12 uv__async_io.part.0
        /work/libuv-1.51.0/src/unix/async.c:208:5
```

Signed-off-by: ndossche <nora.dossche@ugent.be>
PR-URL: nodejs#63476
Reviewed-By: Anna Henningsen <anna@addaleax.net>
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