Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix asan error for new-delete-type-mismatch #1411

Merged
merged 1 commit into from
Jun 19, 2024
Merged

Conversation

satoren
Copy link
Contributor

@satoren satoren commented Jun 19, 2024

This handle seems to be of type uv_pipe_t.

Copy link
Member

@jmillan jmillan left a comment

Choose a reason for hiding this comment

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

indeed handle is of type uv_stream_t*.

@ibc
Copy link
Member

ibc commented Jun 19, 2024

indeed handle is of type uv_stream_t*.

Nope, it's uv_pipe_t*:

err = uv_shutdown(
      req, reinterpret_cast<uv_stream_t*>(this->uvHandle), static_cast<uv_shutdown_cb>(onShutdown));

In .hpp:

uv_pipe_t* uvHandle{ nullptr };

So PR is ok IMHO.

@jmillan
Copy link
Member

jmillan commented Jun 19, 2024

Nope, it's uv_pipe_t*:

I verified that shutdown_t->handle is a uv_stream_t*. Probably that's casted from uv_pipe_t*, but I'm not sure.

If ASAN is not complaining then 👍

@ibc
Copy link
Member

ibc commented Jun 19, 2024

Nope, it's uv_pipe_t*:

I verified that shutdown_t->handle is a uv_stream_t*. Probably that's casted from uv_pipe_t*, but I'm not sure.

If ASAN is not complaining then 👍

uv_pipe is a subtype of uv_stream. uv_tcp as well.

@ibc
Copy link
Member

ibc commented Jun 19, 2024

Let's not merge yet, I want to check something.

@ibc
Copy link
Member

ibc commented Jun 19, 2024

It's ok. Merging. Thanks @satoren. But the same should be done in TcpConnectionHandle.cpp, I will do it in a separate PR.

@ibc ibc merged commit 81e2099 into versatica:v3 Jun 19, 2024
35 checks passed
ibc added a commit that referenced this pull request Jun 19, 2024
This is the same change as done in PR #1411 but here in `TcpConnectionHandle.cpp`.
@ibc
Copy link
Member

ibc commented Jun 19, 2024

It's ok. Merging. Thanks @satoren. But the same should be done in TcpConnectionHandle.cpp, I will do it in a separate PR.

Done here: 08bdf56

dhilipsiva added a commit to ds-forks/mediasoup that referenced this pull request Jun 22, 2024
* v3:
  chore: Update Rust toolchain channel to version 1.79.0
  Fix issue versatica#1374
  cosmetic
  fix Simulcast IncreaseLayer bug when producer score is zero (versatica#1410)
  Update NPM deps
  TcpConnectionHandle.cpp: properly close handle
  fix asan error for new-delete-type-mismatch (versatica#1411)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants