Skip to content

Conversation

@targos
Copy link
Member

@targos targos commented Dec 20, 2025

Following Chromium.

Closes: #61125
Refs: https://issues.chromium.org/issues/388070065

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. labels Dec 20, 2025
@targos targos mentioned this pull request Dec 20, 2025
@targos
Copy link
Member Author

targos commented Dec 20, 2025

Shows an error in quic code (@jasnell):

In file included from /usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/memory:78:
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/unique_ptr.h:91:16: error: invalid application of 'sizeof' to an incomplete type 'node::quic::Stream::Outbound'
   91 |         static_assert(sizeof(_Tp)>0,
      |                       ^~~~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/unique_ptr.h:205:4: note: in instantiation of member function 'std::default_delete<node::quic::Stream::Outbound>::operator()' requested here
  205 |           _M_deleter()(__old_p);
      |           ^
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/unique_ptr.h:503:7: note: in instantiation of member function 'std::__uniq_ptr_impl<node::quic::Stream::Outbound, std::default_delete<node::quic::Stream::Outbound>>::reset' requested here
  503 |         _M_t.reset(std::move(__p));
      |              ^
../src/quic/streams.cc:324:23: note: in instantiation of member function 'std::unique_ptr<node::quic::Stream::Outbound>::reset' requested here
  324 |     stream->outbound_.reset();
      |                       ^
../src/quic/streams.h:294:9: note: forward declaration of 'node::quic::Stream::Outbound'
  294 |   class Outbound;
      |         ^
1 error generated.

@targos
Copy link
Member Author

targos commented Dec 20, 2025

I opened ngtcp2/ngtcp2#1970 for the ngtcp2 error.

@ShenHongFei
Copy link
Contributor

temporary workaround.

Aside from this, there are no other problems, and it compiles successfully.

diff --git a/src/quic/streams.h b/src/quic/streams.h
index c230815d78e..3c9fb100219 100644
--- a/src/quic/streams.h
+++ b/src/quic/streams.h
@@ -292,7 +292,12 @@ class Stream final : public AsyncWrap,
   struct PendingHeaders;
 
   class Outbound;
-
+  
+  
+  struct OutboundDeleter {
+    void operator() (Outbound* ptr) const;
+  };
+  
   // Gets a reader for the data received for this stream from the peer,
   BaseObjectPtr<Blob::Reader> get_reader();
 
@@ -335,7 +340,7 @@ class Stream final : public AsyncWrap,
   AliasedStruct<Stats> stats_;
   AliasedStruct<State> state_;
   BaseObjectWeakPtr<Session> session_;
-  std::unique_ptr<Outbound> outbound_;
+  std::unique_ptr<Outbound, OutboundDeleter> outbound_;
   std::shared_ptr<DataQueue> inbound_;
 
   // If the stream cannot be opened yet, it will be created in a pending state.
diff --git a/src/quic/streams.cc b/src/quic/streams.cc
index 8fe5b72ce1f..8fa9c264c4f 100644
--- a/src/quic/streams.cc
+++ b/src/quic/streams.cc
@@ -739,6 +739,11 @@ class Stream::Outbound final : public MemoryRetainer {
   size_t uncommitted_ = 0;
 };
 
+
+void Stream::OutboundDeleter::operator() (Stream::Outbound* ptr) const {
+    delete ptr;
+}
+
 // ============================================================================
 
 #define V(name, key, no_side_effect)                                           \
@@ -1038,7 +1043,7 @@ void Stream::set_outbound(std::shared_ptr<DataQueue> source) {
   if (!source || !is_writable()) return;
   Debug(this, "Setting the outbound data source");
   DCHECK_NULL(outbound_);
-  outbound_ = std::make_unique<Outbound>(this, std::move(source));
+  outbound_ = std::unique_ptr<Outbound, OutboundDeleter>(new Outbound(this, std::move(source)));
   state_->has_outbound = 1;
   if (!is_pending()) session_->ResumeStream(id());
 }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move to C++23

4 participants