src: minor refactoring to StreamBase by addaleax · Pull Request #17564 · nodejs/node
added 2 commits
This is not necessary since C++ already has `static_cast` as a proper way to cast inside a class hierarchy.
Instead of having per-request callbacks, always call a callback on the `StreamBase` instance itself for `WriteWrap` and `ShutdownWrap`. This makes `WriteWrap` cleanup consistent for all stream classes, since the after-write callback is always the same now. If special handling is needed for writes that happen to a sub-class, `AfterWrite` can be overridden by that class, rather than that class providing its own callback (e.g. updating the write queue size for libuv streams). If special handling is needed for writes that happen on another stream instance, the existing `after_write_cb()` callback is used for that (e.g. custom code after writing to the transport from a TLS stream). As a nice bonus, this also makes `WriteWrap` and `ShutdownWrap` instances slightly smaller.
labels
nodejs-github-bot
added
c++
labels
addaleax
added
the
author ready
label
addaleax added a commit to addaleax/node that referenced this pull request
This is not necessary since C++ already has `static_cast` as a proper way to cast inside a class hierarchy. PR-URL: nodejs#17564 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax added a commit to addaleax/node that referenced this pull request
Instead of having per-request callbacks, always call a callback on the `StreamBase` instance itself for `WriteWrap` and `ShutdownWrap`. This makes `WriteWrap` cleanup consistent for all stream classes, since the after-write callback is always the same now. If special handling is needed for writes that happen to a sub-class, `AfterWrite` can be overridden by that class, rather than that class providing its own callback (e.g. updating the write queue size for libuv streams). If special handling is needed for writes that happen on another stream instance, the existing `after_write_cb()` callback is used for that (e.g. custom code after writing to the transport from a TLS stream). As a nice bonus, this also makes `WriteWrap` and `ShutdownWrap` instances slightly smaller. PR-URL: nodejs#17564 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax added a commit to addaleax/node that referenced this pull request
Check that `serverConnectionHandle.writeQueueSize === 0` after a large write finished. PR-URL: nodejs#17564 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax added a commit that referenced this pull request
This is not necessary since C++ already has `static_cast` as a proper way to cast inside a class hierarchy. PR-URL: #17564 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax added a commit that referenced this pull request
Instead of having per-request callbacks, always call a callback on the `StreamBase` instance itself for `WriteWrap` and `ShutdownWrap`. This makes `WriteWrap` cleanup consistent for all stream classes, since the after-write callback is always the same now. If special handling is needed for writes that happen to a sub-class, `AfterWrite` can be overridden by that class, rather than that class providing its own callback (e.g. updating the write queue size for libuv streams). If special handling is needed for writes that happen on another stream instance, the existing `after_write_cb()` callback is used for that (e.g. custom code after writing to the transport from a TLS stream). As a nice bonus, this also makes `WriteWrap` and `ShutdownWrap` instances slightly smaller. PR-URL: #17564 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax added a commit that referenced this pull request
Check that `serverConnectionHandle.writeQueueSize === 0` after a large write finished. PR-URL: #17564 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax
removed
the
author ready
label
MylesBorins pushed a commit that referenced this pull request
This is not necessary since C++ already has `static_cast` as a proper way to cast inside a class hierarchy. PR-URL: #17564 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Instead of having per-request callbacks, always call a callback on the `StreamBase` instance itself for `WriteWrap` and `ShutdownWrap`. This makes `WriteWrap` cleanup consistent for all stream classes, since the after-write callback is always the same now. If special handling is needed for writes that happen to a sub-class, `AfterWrite` can be overridden by that class, rather than that class providing its own callback (e.g. updating the write queue size for libuv streams). If special handling is needed for writes that happen on another stream instance, the existing `after_write_cb()` callback is used for that (e.g. custom code after writing to the transport from a TLS stream). As a nice bonus, this also makes `WriteWrap` and `ShutdownWrap` instances slightly smaller. PR-URL: #17564 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Check that `serverConnectionHandle.writeQueueSize === 0` after a large write finished. PR-URL: #17564 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Instead of having per-request callbacks, always call a callback on the `StreamBase` instance itself for `WriteWrap` and `ShutdownWrap`. This makes `WriteWrap` cleanup consistent for all stream classes, since the after-write callback is always the same now. If special handling is needed for writes that happen to a sub-class, `AfterWrite` can be overridden by that class, rather than that class providing its own callback (e.g. updating the write queue size for libuv streams). If special handling is needed for writes that happen on another stream instance, the existing `after_write_cb()` callback is used for that (e.g. custom code after writing to the transport from a TLS stream). As a nice bonus, this also makes `WriteWrap` and `ShutdownWrap` instances slightly smaller. Backport-PR-URL: #20456 PR-URL: #17564 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com>
abhishekumar-tyagi pushed a commit to abhishekumar-tyagi/node that referenced this pull request
Instead of having per-request callbacks, always call a callback on the `StreamBase` instance itself for `WriteWrap` and `ShutdownWrap`. This makes `WriteWrap` cleanup consistent for all stream classes, since the after-write callback is always the same now. If special handling is needed for writes that happen to a sub-class, `AfterWrite` can be overridden by that class, rather than that class providing its own callback (e.g. updating the write queue size for libuv streams). If special handling is needed for writes that happen on another stream instance, the existing `after_write_cb()` callback is used for that (e.g. custom code after writing to the transport from a TLS stream). As a nice bonus, this also makes `WriteWrap` and `ShutdownWrap` instances slightly smaller. PR-URL: nodejs/node#17564 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com>