Rename the free function to 'toHash', as no merging was happening, this
name was misleading.
Improve the implementation by adding the missing 'reserve()' and simply
try_emplace() the pair into the hash. This will still take the newest
value as before but without constructing a temporary key list and having
the extra lookup with value().
Pick-to: 6.10
Change-Id: I4834feea43e20614caea3d587055c024012e0596
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
We forgot to deprecate the server-metadata interfaces when we deprecated
the client-metadata. The same problems apply here too!
Found during the 6.10 API-review.
[ChangeLog][Deprecation Notice]
Deprecate the metadata()/serverMetadata()/setServerMetadata() methods on
QGrpcOperation and QGrpcOperationContext that use QHash in favor of the
new server{Initial,Trailing}Metadata interfaces, that use QMultiHash and
provide the correct handling of the received metadata in their
respective phase. This is a behavior change as the old metadata()
interface now only returns the initial metadata.
Fixes: QTBUG-138039
Pick-to: 6.10
Change-Id: I307ee81fc353a0f4316fea2d10f56bb6910ae859
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
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.
Pick-to: 6.10 6.9 6.8
Change-Id: Ic4e70b50afe46b5a883f099a6cf245ea9a0e66c1
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
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
Pick-to: 6.10 6.9 6.8
Change-Id: I8e17f7832659fb348e7d05aeabe164b16b6ff283
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
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.
Pick-to: 6.10 6.9 6.8
Change-Id: Ib1d8a16db65cadfecb242f54a27ab70bb08a78fd
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
createHttp2Stream is calling finished asynchronously. It must report the
outcome back immediately though, as processOperation() depends on it.
Pick-to: 6.10 6.9 6.8
Change-Id: Ic01fea69dba491c8f119eb3a28523878a8e214d7
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
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
Pick-to: 6.10 6.9 6.8
Change-Id: Ifad7230a7592dc5d7691379031356afe1f8f8dc3
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
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
Pick-to: 6.10 6.9 6.8
Change-Id: I0d2aba0f123a408b43dc4eb0f2a045898b41bc96
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
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.
Pick-to: 6.10 6.9 6.8
Change-Id: I1eb58d143e4934a3c0770cd3ff24ed47972a5289
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
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.
Pick-to: 6.10 6.9 6.8
Change-Id: Iee81e98e826f6019fdf525b167faa33944ebadb2
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
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.
Pick-to: 6.10 6.9 6.8
Change-Id: I30b5c935c4fe3aaafb66ed91e25562afca2f8804
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
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
Pick-to: 6.10 6.9 6.8
Change-Id: I095d9ecc3574b8ad1ed344d16701102d2c79df92
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
For consistency. No null checking is needed as the channel will always
destroy all children before destroying itself.
Pick-to: 6.10 6.9 6.8
Change-Id: I4638cd906dc6c66d2559048bd147216518655110
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
Extends the testcase. The call should fail here with a non-OK status
code.
Pick-to: 6.10 6.9 6.8
Change-Id: I4b8f833f2cb147d9b301004c51dc2a3388876fb6
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
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.
Pick-to: 6.10 6.9 6.8
Change-Id: I880846ae06e8db338cdb3629dafdff430cab3edb
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
For later usage in server metadata deprecation.
Pick-to: 6.10
Change-Id: I973943063b1ba0b065bba48abd12a1be008ebed4
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
These functions are not callable. Declaring them const allows them
to match more calls (specifically, const rvalues, which before would
have given "no matching overload" instead of "deleted" errors).
Pick-to: 6.10
Change-Id: I3d074d561456cf98fccacabea68204c92b7a4b38
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
The variable is used unconditionally.
Amends c91d02c007
Pick-to: 6.10 6.9 6.8
Change-Id: I36910a7b40f16a5a13bf2839a2e6c27aec5e7b26
Reviewed-by: Dennis Oberst <dennis.oberst@qt.io>
We store the host protoc and generator paths from crosscompiling
environment when building generator tests. The test is supposed to
run on target meanwhile, which surely will lead to an issue if
paths to the tools on target platform differ. This particulary happens
on windows arm64 machine when we copy the cross-compiled tests.
Pick-to: 6.10 6.9 6.8
Change-Id: I85971da1b429383cae0f842306629355fc5833c6
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>
In some reference libprotobuf/libprotoc installations the include
paths of the protobuf::libprotoc target are missing.
Force using protobuf::libprotobuf as the dependency for the
libprotoc compile test.
Change-Id: I56bb84910bc178063d42292f04d7e75c04e9ca4d
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
Reviewed-by: Dennis Oberst <dennis.oberst@qt.io>
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>
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.10 6.9 6.8
Change-Id: I114f58d1d443164eb419dab98623d06044419a81
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
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.10 6.9 6.8
Change-Id: I4eb7f6fbc1474508af2a88a3313cf94325a63657
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
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
Pick-to: 6.10 6.9
Change-Id: I7d0491684af8c651cafb988acfac311f148a71b6
Reviewed-by: Kai Köhne <kai.koehne@qt.io>
The \c prevents the macro to resolve. Anyhow, \c should be used only
for command tools etc, not for generic product names. Just drop it.
Fixes: QTBUG-138179
Change-Id: Id5f10242ff0218fb6bc3cc4771fab722bdbc07e2
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
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.10 6.9 6.8
Change-Id: I9416de25169fbaaa1657db7120987754699e4c00
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
Reviewed and marked all security critical files in the
qtgrpc/src/tools directory.
More information: https://contribute.qt-project.org/quips/23
Pick-to: 6.10
Task-number: QTBUG-135456
Change-Id: I1328431d24aa330a40e95824fdcdbb34e6cf53cd
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
Reviewed and marked all security critical files in the
qtgrpc/src/protobuf directory.
More information: https://contribute.qt-project.org/quips/23
Fixes: QTBUG-135456
Pick-to: 6.10
Change-Id: I9cc05039ca5830d04816ada978ec193ece154732
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
Decouples our public API from the implementation. This is also slightly
superior as potential duplicates will not be copied on function call.
Discovered during the 6.10 API review.
Pick-to: 6.10
Change-Id: I78bc123cf4db034ce12f62a5a4576c8e0d0d64f3
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
... to avoid qdoc warnings about it being undocumented.
Pick-to: 6.10
Change-Id: Ic092c26d7e43dd9a51646cc6edf6f418bc8857ec
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
Avoid using improper default-constuctable enum class, use the
exising API for the tag definition instead.
Pick-to: 6.10
Change-Id: Id75cb7e06a54139116595c60ec09f3810313f26b
Reviewed-by: Dennis Oberst <dennis.oberst@qt.io>
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
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 6.9 6.10
Change-Id: I1e907e7be26048174899855efc6a9ed661a1e4d0
Reviewed-by: Tatiana Borisova <tatiana.borisova@qt.io>
Dennis Oberst
Qt Submodule Update Bot
Alexey Edelev
Lucie Gérard
Marc Mutz
Alexei Cazacov
Kai Köhne
Cristian Le
Jani Heikkinen