Removed redundant 'QObject::' namespace to streamline code and enhance
readability, especially in lambda handlers.
Due to our formatting, all lambda handlers will be unnecessarily aligned
to the very right side. Given that connections and lambda handlers are
very common in this codebase this improves the readability of those
lambda handlers.
Change-Id: I178a838c7702382b4b3845c729d7c11eeeb1c8d1
Reviewed-by: Tatiana Borisova <tatiana.borisova@qt.io>
(cherry picked from commit eb703a45a1)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 2720a3e74d)
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
(cherry picked from commit 7a3ebbc1f9)
Easily allows to have a deeper inspection of the system by adding more
debug prints, which can be enabled with the logging category.
Change-Id: I86d7f6c0c53c412a79a3d66f515fa1cd6757a023
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
(cherry picked from commit 7e06d79d6d)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit f48600b44b)
(cherry picked from commit b214760208)
Given the complexity of the http2channel implementation it comes natural
to extend its logging clarity. This aligns with best practices in Qt
development. Also this makes it easier to debug the system remotely.
Change-Id: Idf00020408b678fc6b17a25db538abd5a838bced
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
(cherry picked from commit f1200375f0)
(cherry picked from commit abb6a173de)
(cherry picked from commit 5bf8f9ace3)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
1. Remove nesting by reversing the state check.
2. Split the messageReceived calculation to be outside of the function
call. Given that this is an important calculation it should be easily
understandable.
Change-Id: Icfaa29a5dc92eb5b1c86469639a3deb90b5aacd0
Reviewed-by: Tatiana Borisova <tatiana.borisova@qt.io>
(cherry picked from commit 4504e44869)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 954c3bd94d)
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
(cherry picked from commit 11efb10198)
Re-order the argument to the Http2Handler ctor. The parent should come
first. Furthermore rename the m_operation to m_context as I think
context is a more fitting name for this important member.
This improves the readability and makes it easier to follow the code.
Change-Id: I396e205ec345d80a8cf2cfebe43625f72d39ac6e
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
(cherry picked from commit 1ac37b787a)
(cherry picked from commit a3c497adf3)
(cherry picked from commit 9a08967617)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
Currently, the QGrpcOperation was finishing itself so that the channel
implementation should only cancel the corresponding RPC. This is the
only place such self-finishing is used.
Let the channel implementation rather take care of the cancellation
logic and emit the finished signal. The actual handler implementation is
responsible for itself, i.e. finished meaning it's done and no
communication will happen through this stream.
Simplify the logic for timeouts and requested cancellations.
Update the deadline testcase to not check for specific messages but
rather for non-empty messages.
Changes will only be relevant for custom channel implementation.
[ChangeLog][QGrpcOperationContext][Important Behavior Changes]
Cancellation logic should also emit finished now. Custom
QAbstractGrpcChannel implementations should adapt their logic.
Change-Id: Ic4e70b50afe46b5a883f099a6cf245ea9a0e66c1
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
(cherry picked from commit 814c0b6ac2)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit c1b7403996)
(cherry picked from commit bc7de6f42f)
Our lifetime management was extremely flawed, as there were many points
where Http2Handler instances were not properly deleted, resulting in
inactive zombie handlers that would never be cleaned up.
In a running event loop, there should be no active or pending handlers
left at the time of channel destruction. If we had previously asserted
this condition:
QGrpcHttp2ChannelPrivate::~QGrpcHttp2ChannelPrivate()
{
Q_ASSERT(children().isEmpty());
}
and then ran our tests, many cases would reveal that we were effectively
leaking memory due to inproper lifetime management on our side.
This is fixed by applying the following:
When the finished signal is emitted, the corresponding Http2Handler
should be deleted. It no longer makes sense to keep it alive beyond that
point, as this aligns with our documented lifetime for client-side RPC
handlers.
Transform the operation context into a QPointer to not steady convert
into shared_ptr's. Connect to the destroyed signal to keep track when
the user-side handler gets deleted.
We definitely don't want to take part in sharing the ownership (and
therefore lifetime) of the operationContext. Pass down a pointer very
early on so that no mistakes happen in the future (as to take a copy of
the shared_ptr in a lambda).
This is streamlined by introducing the finish and asyncFinish functions.
Task-number: QTBUG-128338
Fixes: QTBUG-129160
Change-Id: I8e17f7832659fb348e7d05aeabe164b16b6ff283
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
(cherry picked from commit 3fea4be230)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit d381fcfef2)
(cherry picked from commit 0a8ad4fc23)
The `m_connection` check was not needed as there are two places where
this is called:
- inside createHttp2Connection() -> connection gets created, 100% valid
- inside processOperation(), where we need the m_connection check
anyways.
Remove the check + error and replace it with an assert.
Change-Id: Ib1d8a16db65cadfecb242f54a27ab70bb08a78fd
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
(cherry picked from commit 5e969e8646)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit fa07b01070)
(cherry picked from commit bcade980cc)
createHttp2Stream is calling finished asynchronously. It must report the
outcome back immediately though, as processOperation() depends on it.
Change-Id: Ic01fea69dba491c8f119eb3a28523878a8e214d7
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
(cherry picked from commit e48296e650)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 9ce9ab229c)
(cherry picked from commit 032ae28866)
The QGrpcOperation::cancel() logic is already emitting
QGrpcOperation::finished() for us but the previous cancellation logic
was not deleting the cancelled handler.
Fix this by deleting unconditionally. This is fine for cancelled
handlers. Furthermore transform the cancel() function to return void.
Cancellation should be used unconditionally.
Task-number: QTBUG-129160
Change-Id: Ifad7230a7592dc5d7691379031356afe1f8f8dc3
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
(cherry picked from commit e232a4ee5e)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 0a8559c3f6)
(cherry picked from commit 9524471516)
The QHttp2Stream implementation doesn't provide any handling or
verification of the HTTP/2 spec. The gRPC over HTTP/2 protocol clearly
defines how Response-Headers, Trailers and Trailers-Only have to behave.
Currently we are missing crucial steps of validation, like:
- HTTP status to be 200
- Trailers contain gRPC status
- Valid gRPC content-type found
Ref: https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#responses
Create the handleHeaders() function to correctly handle the protocol.
Correctly differentiate between Initial, Trailers and TrailersOnly now.
Fixes: QTBUG-138494
Change-Id: I0d2aba0f123a408b43dc4eb0f2a045898b41bc96
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
(cherry picked from commit 9de5085814)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 3a2caf9ece)
(cherry picked from commit f48fb557ca)
We should start the deadline-timer when the actual call is beginning and
this is indicated when the initial headers have been sent and not the
stream that has been attached. As a drive-by mark the timer as
single-shot.
Change-Id: I1eb58d143e4934a3c0770cd3ff24ed47972a5289
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
(cherry picked from commit cdc6938bbf)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 2f2649fa20)
(cherry picked from commit 90209b70ba)
The deletion was invalid and could result in a double free. If
createHttp2Connection() encountered expired handlers, it deleted them
but did not remove them from the container. Later, when
settingsFrameReceived() iterated over the same container, it would
encounter the already-deleted handler again.
Harden our handler management by relying on QObject parent <> child
relationship instead of additionally tracking the children by our own.
The handler (child) was already doing that. The channel didn't actually
need any active or pending containers. It's enough to handle the
processing through QObject::children().
We have to add the Q_OBJECT macro now to fully support parent <> child
relationship.
Change-Id: Iee81e98e826f6019fdf525b167faa33944ebadb2
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
(cherry picked from commit 8d58b8ba6a)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit c0b3a93937)
(cherry picked from commit 21becf8b31)
Since the Http2Handler is largely self-managed, having a clear and
accurate state representation is crucial.
Previously, some key states were missing—particularly those relevant to
the header phase. This made it harder to ensure correct behavior and
state transitions.
This change introduces the 'Idle' and 'RequestHeadersSent' states to
better reflect the handler’s lifecycle. With this we can enforce more
robust validation and ensure the handler behaves as expected.
Furthermore we change the onDone() handling logic (to only allow it to
be called once) to use a bool. Using state management for this is
impractical. It's not a real state of our Http2Handler. It's leaking out
the internal stream state.
Change-Id: I30b5c935c4fe3aaafb66ed91e25562afca2f8804
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
(cherry picked from commit 76b8c2626a)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 3f5d1aa65c)
(cherry picked from commit 3e8b231008)
This patch follows C++ best practices. Refactor the
'prepareInitialRequest' function to directly return the constructed
headers so that we can use them in the initializer list.
Ref: https://isocpp.org/wiki/faq/ctors#init-lists
Change-Id: I095d9ecc3574b8ad1ed344d16701102d2c79df92
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
(cherry picked from commit 46bce56bfb)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit c2232f75b4)
(cherry picked from commit e577763fb4)
For consistency. No null checking is needed as the channel will always
destroy all children before destroying itself.
Change-Id: I4638cd906dc6c66d2559048bd147216518655110
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
(cherry picked from commit f8196f8050)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit b5e89d251e)
(cherry picked from commit 3002f4ce82)
Extends the testcase. The call should fail here with a non-OK status
code.
Change-Id: I4b8f833f2cb147d9b301004c51dc2a3388876fb6
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
(cherry picked from commit ef9ff0cd55)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit a3c69b53fb)
(cherry picked from commit a71b2691c9)
Extend the testcase to cover both, initial and trailing metadata. This
also shows the current problems with having a QHash metadata and also
the missing separation between initial and trailing metadata.
Change-Id: I880846ae06e8db338cdb3629dafdff430cab3edb
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
(cherry picked from commit 542efe69ec)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 8dc879a824)
(cherry picked from commit a6fef73f28)
The variable is used unconditionally.
Amends c91d02c007
Pick-to: 6.8
Change-Id: I36910a7b40f16a5a13bf2839a2e6c27aec5e7b26
Reviewed-by: Dennis Oberst <dennis.oberst@qt.io>
(cherry picked from commit 740c809f94)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit bb1c65de03)
qmatrix4x4.h will lose its qquaternion.h include, so include
qquaternion.h explicitly in all files that mention 'QQuaternion',
unless, for a foo.cpp, the own foo.h has already included it.
Amends 6637773cbd.
Pick-to: 6.8
Change-Id: I114f58d1d443164eb419dab98623d06044419a81
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
(cherry picked from commit 944aa03dea)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 487f87c78c)
This commit removes the latest example entry from the TOC, because it
somehow breaks the tree generation. The example entry is not needed
here, because the examples are added to the TOC automatically.
Fixes: QTBUG-138180
Change-Id: I7d0491684af8c651cafb988acfac311f148a71b6
Reviewed-by: Kai Köhne <kai.koehne@qt.io>
(cherry picked from commit 837145416f)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 1280f84db4)
This patch includes:
- Show available server ports dynamically. Qt built without SSL
support should not show the https port.
- Add an early note about the prerequisites for running the example so
that users are guided from early on.
- Use std::cout in combination with std::endl to flush the buffer.
QtCreator doesn't print when simply using "\n".
Pick-to: 6.8
Change-Id: I4eb7f6fbc1474508af2a88a3313cf94325a63657
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
(cherry picked from commit e81b5d59ad)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 9bbd8d17fc)
Instead of storing the build-time path to the well-known types
protobuf includes, calculate them when configuring user projects.
New approach expects that all well-known type schemas that the
ProtobufWellKnownTypes module used at build time are present when
configuring user projects. The include paths then calculated and
stored in the QT_PROTOBUF_INCLUDES property of the
ProtobufWellKnownTypes module.
The property usage remains unchanged.
Pick-to: 6.8
Fixes: QTBUG-130113
Change-Id: I31a607404f85f2a325c63e0f28d2ab2a0f4ea25f
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>
(cherry picked from commit a445cf4344)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
Add the TYPES argument to the qt_internal_add_protobuf_wellknown_types
command instead of using ARGN. It's safer and more scalable approach.
Pick-to: 6.8
Change-Id: I75ac5df740b614433b74e8b70729211f49e0a308
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>
Reviewed-by: Dennis Oberst <dennis.oberst@qt.io>
(cherry picked from commit 5494d64908)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
Iterate the _<Module>_proto_external_include_dir variable in
<Module>ProtobufProperties.cmake and add the existing paths to
the QT_PROTO_INCLUDES property. This allow specifying extra paths
in <Module>Config.cmake.
If the respecive paths are not found or are not absolute, the
respective warning is raised.
Setting QT_NO_WARN_PROTOBUF_BROKEN_INCLUDES to ON suppresses the
warning.
Pick-to: 6.8
Change-Id: I4b6971e84f35b2986d187fb5d1766ed0cf3390f5
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>
(cherry picked from commit 18bb2ba5e1)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
Go up the chain and use the highest abstraction of the sendDATA
interfaces. Previously, we've been hit by internal QObject::deleteLater
calls for destroying the device in QtNetwork, which slowed down the
sending process. This has been fixed upstream, and all sendDATA
calls, except the QNCBD overload, now destroy the device immediately.
Reference: 4bc878ff4fbacd39d4c0ed1e8e742fd18fa74fed.
This version is now superior, as we don't have to create a new
connection just to destroy the device.
Pick-to: 6.8
Change-Id: I9416de25169fbaaa1657db7120987754699e4c00
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
(cherry picked from commit 7bac7883b7)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 0da4fadb73)
Sanitize proto includes to ensure the generated
<Module>ProtobufProperties.cmake is fully relocatable.
Drive-by, ensure that includes do not contain duplicates.
Task-number: QTBUG-130113
Pick-to: 6.8
Change-Id: I64f56d497d412705f174a027f711b90ad7614abf
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>
(cherry picked from commit b8b8dd9fe2)
The prefix was wrongly added to the parent directory but not to the
actual file name.
Amends d4e8ef8942
Fixes: QTBUG-137313
Pick-to: 6.8
Change-Id: I1e907e7be26048174899855efc6a9ed661a1e4d0
Reviewed-by: Tatiana Borisova <tatiana.borisova@qt.io>
(cherry picked from commit 9e4b2ec356)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 0505c39ba5)
The existing benchmarks were not optimal: bidirectional streaming wasn’t
fully asynchronous, limiting channel performance. Introduce a
`BenchmarkData` type to collect detailed metrics and print richer
results at the end. Also add latency measurements for unary READ and
WRITE operations.
As a drive-by add cxx20 checks in cmake.
Pick-to: 6.8
Change-Id: I56d61e4712b7fe9e5b8b52a14f84b0583094e373
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
(cherry picked from commit bfb44df850)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>