QNetworkRequest ownership of QUrl object?
-
I'm a little confused as to exactly what the QNetworkRequest object does with the QUrl passed to it by reference. My initial assumption was that it takes ownership of the object; indeed, looking at the documentation for the constructor that takes a QUrl,
@QNetworkRequest::QNetworkRequest ( const QUrl & url = QUrl() )@
there would presumably have to be a memory leak if QNetworkRequest didn't take ownership when constructed with no arguments.
However, I can't find anything in the documentation that says QNetworkRequest takes ownership of the object, and I'm more used to seeing objects passed by pointer rather than by reference in the situations where Qt objects do take over ownership... Am I missing something here? Thanks!
-
Programmer should take care of releasing the URL. Logically it can't take the ownership of the same. It is not one-to-one mapping between the QNetworkReqeust and URL. You should have one NetworkRequest object and change the every QURL every time you make network call. You destroy QURL once fetch is complete. Ideally this goes with any argument passing. Programmer need to take care of the lifetime of the object passed to it.
-
Hi,
[quote author="Copernicus" date="1403643899"]I'm a little confused as to exactly what the QNetworkRequest object does with the QUrl passed to it by reference.
...
there would presumably have to be a memory leak if QNetworkRequest didn't take ownership when constructed with no arguments.[/quote]Notice that it is passed by const reference. The QNetworkRequest is not allowed to modify it or free its memory.
Are you familiar with the differences between passing-by-pointer and passing-by-value? passing-by-const-reference is like passing-by-value, just more efficient. This is because passing-by-value creates a new copy of the data, but passing-by-const-reference doesn't.
Anyway, since the QUrl was not created using new, there is no need to delete it.
@
void function() {
// Create a QUrl on the stack, as a local variable
QUrl url1("http://www.qt-project.org/")// "Copy" url1 into the QNetworkRequest QNetworkRequest req(url1); doOtherStuff(); // url1 is automatically destroyed when this function returns
}
@Similarly, the default-constructed QUrl is automatically destroyed when the QNetworkRequest constructor returns.
[quote author="Copernicus" date="1403643899"]However, I can't find anything in the documentation that says QNetworkRequest takes ownership of the object, and I'm more used to seeing objects passed by pointer rather than by reference in the situations where Qt objects do take over ownership...[/quote]A QUrl is a "value object", so nobody "owns" of it.
You are used to using "identity objects", which are the opposite of "value objects" -- each identity object should have exactly one owner. See "Qt Objects|Identity vs. Value":http://qt-project.org/doc/qt-5/object.html#qt-objects-identity-vs-value for further discussion.
Other examples of value types are int, bool, QString, QNetworkRequest (yes, QNetworkRequest too)
[quote author="Dheerendra" date="1403666653"]Programmer should take care of releasing the URL.[/quote]Wrong. See above.
-
Yes, You are right JKSH. It is my oversight.
-
Ah, thank you both, for some reason I'd gotten it into my head that the default QUrl was being constructed on the heap, not the stack. :)
Still, this kind of makes me wonder what QNetworkRequest is doing with it, as the QUrl should go out of scope once the constructor finishes, shouldn't it?
Anyway, my problem was that I was thinking of keeping around a single QUrl object for multiple requests, giving it a new url each time by using the setUrl() method. Of course, this is not a good idea if the QNetworkRequest is using the object in an asynchronous call...
[quote author="JKSH" date="1403669032"]Hi,
Are you familiar with the differences between passing-by-pointer and passing-by-value? passing-by-const-reference is like passing-by-value, just more efficient. This is because passing-by-value creates a new copy of the data, but passing-by-const-reference doesn't.[/QUOTE]Well, the problem here is that the QNetworkAccessManager methods (such as get(), the one I'm currently using) work asynchronously. This is not a problem for data passed by value, but passing by reference means that I could in theory modify the QUrl on my side while it is still being used by the QNetworkRequest object. That's what I'm worried about.
[QUOTE]A QUrl is a "value object", so nobody "owns" of it.
You are used to using "identity objects", which are the opposite of "value objects" -- each identity object should have exactly one owner. See "Qt Objects|Identity vs. Value":http://qt-project.org/doc/qt-5/object.html#qt-objects-identity-vs-value for further discussion.[/QUOTE]
Thank you, I do also forget about the special attributes of QObjects quite often. :) Still, my understanding is that the special properties of an "identity" object simply means that each of those objects must always be registered with a unique Qt identifier (and, therefore, you "clone" rather than "copy" them). However, this has nothing to do with ownership; "value" or "identity", any object constructed on the heap needs to eventually be "delete"d by someone...
-
You're welcome :)
[quote author="Copernicus" date="1403688864"]the QUrl should go out of scope once the constructor finishes, shouldn't it?[/quote]That's correct, but that's not a problem. See below.
[quote author="Copernicus" date="1403688864"]I was thinking of keeping around a single QUrl object for multiple requests, giving it a new url each time by using the setUrl() method.[/quote]It doesn't quite work like that.
QNetworkRequest doesn't assume that the original QUrl will hang around in its original state forever, precisely because of the dangers you've described. If it needs to keep the URL, it will create a copy. (In fact, this is exactly what happens. See the "source code":http://code.woboq.org/qt5/qtbase/src/network/access/qnetworkrequest.cpp.html#_ZN15QNetworkRequestC1ERK4QUrl yourself)
Anyway, QUrl is a tiny object, AND it is "implicitly shared":http://qt-project.org/doc/qt-5/implicit-sharing.html. You don't need to optimize your code by sharing the QUrl, because Qt already does that behind the scenes.
[quote author="Copernicus" date="1403688864"]Well, the problem here is that the QNetworkAccessManager methods (such as get(), the one I'm currently using) work asynchronously. This is not a problem for data passed by value, but passing by reference means that I could in theory modify the QUrl on my side while it is still being used by the QNetworkRequest object. That's what I'm worried about.[/quote]Nothing to worry about :) QNetworkRequest keeps a private copy, so feel free to do whatever you want with the original.
@
QUrl url1("http://www.qt-project.org/")
QNetworkRequest req(url1);// QNetworkRequest isn't bothered by the next line;
// it already made a copy of "http://www.qt-project.org/"
url1 = "http://www.google.com/"
@The same principle applies to all the other classes: Even if you pass in a reference, the recipient will take precautions and make a copy if necessary. Qt engineers are aware of the dangers you described, so they designed the classes to be safe and robust.
[quote author="Copernicus" date="1403688864"]my understanding is that the special properties of an "identity" object simply means that each of those objects must always be registered with a unique Qt identifier (and, therefore, you "clone" rather than "copy" them).[/quote]The concept of "identity" isn't a special Qt thing. See this ".NET example":http://codebetter.com/davidhayden/2005/02/22/object-identity-vs-object-equality-overriding-system-object-equalsobject-obj/. The only "unique identifier" involved is the object's memory address.
"Identity" and "value" aren't actually language features, but rather how we think of and use these objects.
[quote author="Copernicus" date="1403688864"]However, this has nothing to do with ownership; "value" or "identity", any object constructed on the heap needs to eventually be "delete"d by someone...[/quote]Yes, you're right.
Nonetheless, it's useful to know the Qt API's style/conventions: Identity objects are always passed-by-pointer, whereas value objects are (almost) always passed-by-const-reference and returned-by-value.
If you follow Qt's style, you'll never need to call delete on Qt's value objects. You can choose to heap-allocate Qt's value objects if you want, but Qt's API never expects you to do that.
-
[quote author="JKSH" date="1403695532"]QNetworkRequest doesn't assume that the original QUrl will hang around in its original state forever, precisely because of the dangers you've described. If it needs to keep the URL, it will create a copy. (In fact, this is exactly what happens. See the "source code":http://code.woboq.org/qt5/qtbase/src/network/access/qnetworkrequest.cpp.html#_ZN15QNetworkRequestC1ERK4QUrl yourself)[/quote]
Ah, thank you! (And yeah, I really should just look at the code myself, I don't take advantage of Qt's open source nearly enough! :) )
But yeah, it's the pass-by-reference that throws me here. The only advantage of pass-by-reference is that you don't have to copy the object being passed; that's why it's efficient. As a long-time C++ programmer, my immediate assumption when seeing something passed by reference is that you aren't going to make a local copy of it. In fact, as you say,
[quote]Anyway, QUrl is a tiny object, AND it is "implicitly shared":http://qt-project.org/doc/qt-5/implicit-sharing.html.[/quote]
Which means it is entirely safe and efficient to pass this object by value. :)
I've gotta say, I still think passing by reference here is misleading. At the very least, it should be mentioned explicitly in the docs whenever a copy is going to be made and the reference thrown away rather than kept. (I doubt that every Qt method that takes a reference will immediately make a local copy of the data; in the general case, that's a very expensive way to pass data around...)
[quote]Nonetheless, it's useful to know the Qt API's style/conventions: Identity objects are always passed-by-pointer, whereas value objects are (almost) always passed-by-const-reference and returned-by-value.[/quote]
Yeah, I guess that's the thing; in C++, I'm used to passing "value" objects by value, period. You never pass a std::string object by reference, unless you want the callee to modify the caller's string...
-
[quote author="Copernicus" date="1403698716"]
Yeah, I guess that's the thing; in C++, I'm used to passing "value" objects by value, period. You never pass a std::string object by reference, unless you want the callee to modify the caller's string...[/quote]Uh... opinions seem to vary. I would be surprised to see a function that takes a const reference and then stores a pointer to the referenced object (unless the function name (or documentation) clearly suggests this, e.g. "storeReferenceTo(const Object& object)"). Also, the fact that the function copies the arguments is an implementation detail that you should not be concerned with. In your string example, maybe the called function copies the string to a C array? Maybe it only copies a substring. Maybe it transforms the data in some other way. In any of these cases, creating a copy by passing by value is wasteful (it may not be horrible with copy-on-write implementations, but still).
I usually assume that a function does not store a reference (/pointer) to an argument unless the documentation says so.
-
[quote author="mmoll" date="1403702742"]Uh... opinions seem to vary.[/quote]
Yeah, I guess I'm mostly just griping here. :)
[quote]I would be surprised to see a function that takes a const reference and then stores a pointer to the referenced object (unless the function name (or documentation) clearly suggests this, e.g. "storeReferenceTo(const Object& object)").[/quote]
Well, but the QNetworkRequest constructor is taking a reference to an object that contains information it will most certainly need after the constructor finishes executing. Therefore, it must either store a pointer to the object, or make a copy of the object's data...
If the object was being passed by value, there wouldn't be any question: the QNetworkRequest constructor wouldn't even be able to store a pointer.
[quote]Also, the fact that the function copies the arguments is an implementation detail that you should not be concerned with.[/quote]
Perhaps that is normally the case, but not here, because this involves asynchronous execution. Let us assume that the QNetworkRequest object did not make a copy of the QUrl object, but rather stored a pointer to it. I construct a QNetworkRequest object using my QUrl object, and pass it to the QNetworkAccessManager in a get() call. The QNetworkAccessManager starts to retrieve data, and meanwhile, control passes back to my application. Some time later, I modify my QUrl object and make another call to get(). Let's say the first call to get() now finishes; I now have a QNetworkReply object. If I want to determine which request this reply is associated with, I can make a call to "request()", to get a copy of the original QNetworkRequest object. But, since it has a pointer (not a copy) of the QUrl object, and I modified that object, the url it returns is not the one that was used during the request!
Even worse, if I deleted the QUrl while the get() was in progress, there'd be a bad pointer floating around...
So, in certain cases, it does require the concern of the programmer.
[quote]I usually assume that a function does not store a reference (/pointer) to an argument unless the documentation says so.[/quote]
It appears that you may be safe in the world of Qt, but take caution elsewhere... ;)
-
[quote author="Copernicus" date="1403705285"]
[quote]Also, the fact that the function copies the arguments is an implementation detail that you should not be concerned with.[/quote]Perhaps that is normally the case, but not here, because this involves asynchronous execution. Let us assume that the QNetworkRequest object did not make a copy of the QUrl object, but rather stored a pointer to it. I construct a QNetworkRequest object using my QUrl object, and pass it to the QNetworkAccessManager in a get() call. The QNetworkAccessManager starts to retrieve data, and meanwhile, control passes back to my application. Some time later, I modify my QUrl object and make another call to get(). Let's say the first call to get() now finishes; I now have a QNetworkReply object. If I want to determine which request this reply is associated with, I can make a call to "request()", to get a copy of the original QNetworkRequest object. But, since it has a pointer (not a copy) of the QUrl object, and I modified that object, the url it returns is not the one that was used during the request!
[/quote]I completely understand the consequences of storing a pointer. The point I was trying to make is that QNetworkRequest needn't necessarily store a copy of a QUrl object, and that you should not need to know. Maybe it resolves the URL right away and only stores an IP address, maybe it converts the QUrl object into whatever format is more suitable for name resolution...
Furthermore, when you use polymorphic types, you basically /must/ pass references (so to be consistent, why not always do it?)
[quote author="Copernicus" date="1403705285"]
So, in certain cases, it does require the concern of the programmer.
[/quote]And whenever it does, the behavior should be clearly and well documented. After all, more questions arise when a pointer is stored, such as: who owns the object? at what point will the object be destroyed? am I even allowed to ever touch the object again (another thread may be concurrently accessing it)?
The same questions also arise when the object contains a reference to some shared resource internally (let's say a file handle). You absolutely need documentation at that point. -
[quote author="mmoll" date="1403710191"]The point I was trying to make is that QNetworkRequest needn't necessarily store a copy of a QUrl object, and that you should not need to know.[/quote]
Maybe in Qt; but in general C/C++, I think you really do need to know. I've given the callee a reference to an object I own; I need to know if the callee is going to need access to that object beyond the life of the function call. If so, that significantly alters what I can safely do with that object. That's all I'm really griping about here; the Qt constructor is safe only because of a Qt coding convention. I think that convention should be noted explicitly when it is used.
[quote][quote author="Copernicus" date="1403705285"]
So, in certain cases, it does require the concern of the programmer.
[/quote]And whenever it does, the behavior should be clearly and well documented.
[/quote]EXACTLY!!! :) :)
-
[quote author="Copernicus" date="1403698716"]I've gotta say, I still think passing by reference here is misleading. At the very least, it should be mentioned explicitly in the docs whenever a copy is going to be made and the reference thrown away rather than kept.
...
It appears that you may be safe in the world of Qt, but take caution elsewhere... ;)
[/quote]But it's not possible for a (parameter) reference to last beyond the lifetime of the function. Unlike pointers, you cannot store a reference.(Technically, constructors can store const references, but anyone who writes a constructor like this -- especially in a library -- deserves to be smacked.)
Passing input parameters by const reference is extremely common in C++; it's not a special Qt thing. Here are some example advocates:
- "Bjarne Stroustrup":http://www.stroustrup.com/bs_faq2.html#call-by-reference (the father of C++) himself
- "Google engineers":http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Reference_Arguments
- This "C++ API design book":http://books.google.com/books?id=IY29LylT85wC&pg=PA210&lpg=PA210&dq=c+++input+"const+reference"
[quote]in C++, I'm used to passing "value" objects by value, period. You never pass a std::string object by reference, unless you want the callee to modify the caller's string...[/quote]
It's perfectly fine to stick to pass-by-value only if that's what you want, but do realize that myFunc(const std::string& str) is "very common":http://stackoverflow.com/questions/10231349/are-the-days-of-passing-const-stdstring-as-a-parameter-over (for good reasons too).In C++98/C++03, passing input arguments by const reference was a no-brainer. The only reason people are now starting to consider pass-by-value over pass-by-const-reference is to take advantage of C++11's move semantics.
[quote author="Copernicus" date="1403711341"][quote][quote author="Copernicus" date="1403705285"]
So, in certain cases, it does require the concern of the programmer.
[/quote]And whenever it does, the behavior should be clearly and well documented.
[/quote]EXACTLY!!! :) :)[/quote]And Qt does just that. :)
If the developer needs to worry about the lifetime of an object, the documentation does say so. For example, you can get a pointer to the internal data of QVector using QVector::data(). The documentation does warn that the pointer could be invalidated when the data changes -- http://qt-project.org/doc/qt-5/qvector.html#data
-
[quote]Passing input parameters by const reference is extremely common in C++; it's not a special Qt thing. Here are some example advocates:
- "Bjarne Stroustrup":http://www.stroustrup.com/bs_faq2.html#call-by-reference (the father of C++) himself[/quote]
I should hope that Stroustrup would advocate the use of a fundamental language feature! But, if I may, let me bring in a quote from the link you just gave:
[quote]Should I use call-by-value or call-by-reference?
That depends on what you are trying to achieve:
If you want to change the object passed, call by reference or use a pointer; e.g. void f(X&); or void f(X*);
If you don't want to change the object passed and it is big, call by const reference; e.g. void f(const X&);
Otherwise, call by value; e.g. void f(X);[/quote]Therefore, Stroustrup himself says that it makes sense to use a reference if (a) you want to change the object being passed, or (b) the object is so big you don't want to make a copy of it. But in this case, the object (a QUrl) is tiny, and is not going to be changed. Passing the object by const reference here has no advantage over calling by value, and adds the complication that the QNetworkAccessManager is pointing to an object that could be modified by the caller during its lifetime. This complication does not occur for pass by value.
Only by immediately making a copy of the object pointed to by the reference does the QNetworkAccessManager remain safe. And Qt does not indicate in the documentation that this copy is made...
-
Hi,
There are multiple topics under contention here, which I'll discuss one at a time:
1. (Main topic) Qt should document the fact it is safe to destroy/modify a QUrl that has been passed into the QNetworkRequest constructor
I'm not convinced this is necessary, because Qt already documents cases where the developer needs to ensure that the object is not destroyed/modified, e.g. the "QLatin1String constructor":http://qt-project.org/doc/qt-5/QLatin1String.html#QLatin1String. In your case, the developer has nothing to worry about, so there's nothing to document.
I'm not the authority on Qt documentation though, so please feel free to submit a request to http://bugreports.qt-project.org/ if you wish (Create a new Issue and label it a "Suggestion"), or submit your own patch.
2. "QNetworkAccessManager [can point] to an object that could be modified by the caller during its lifetime"
How?If you think about how references work, you'll realize that a reference can't last beyond the lifetime of the function call (unless you implement hacks which no self-respecting library should do -- so let's exclude this possibility). A class method can store a pointer, but it can't store a reference.
Yes, QNetworkAccesManager::get() takes a reference and starts an asynchronous operation, but that reference ceases to exist when get() returns (before the operation finishes). Therefore QNetworkAccessManager can't possibly hang on to the QNetworkRequest&.
There's even less to worry about for the QNetworkRequest constructor from your original post, because this function is fully synchronous.
3. "Passing the object by const reference here has no advantage over calling by value"
Definitely disagree.- "Cheap" != "Free"
- "Big" depends on context. If the function is called once only, 1000 bytes is small. But if the function is called repeatedly in a tight loop, 10 bytes is big.
- Qt is a library that is used in many areas, including scientific computation, so its API needs to be designed to accommodate the 10-bytes-is-big use cases.
** Furthermore, passing some objects by-value and others by-reference reduces API cleanness.
Take this example program, which simulates a computationally-intensive operation:
@
#include <QDebug>
#include <QStringList>
#include <QElapsedTimer>int count;
static void addByConstRef(const QString& str) {
count += str.length();
}static void addByValue(QString str) {
count += str.length();
}int main(int, char**) {
// Init, including a list of 10M strings qint64 time; QElapsedTimer t; QStringList list; for (int i = 0; i < 10000000; ++i) list << QString::number(i); // Pass by const reference count = 0; t.start(); for (const QString& str : list) { addByConstRef(str); } time = t.elapsed(); qDebug() << "Const Ref Result:" << count; qDebug() << "Time (ms):" << time << "\n"; // Pass by value count = 0; t.start(); for (QString str : list) { addByValue(str); } time = t.elapsed(); qDebug() << "Value Result:" << count; qDebug() << "Time (ms):" << time << "\n"; return 0;
}
@Using MSVC 2013 on a 3 GHz Pentium D (Win 7 32-bit), this is the output:
@
Const Ref Result: 68888890
Time (ms): 105Value Result: 68888890
Time (ms): 1661
@Copying a QString is cheap, but passing a QString by const reference is still 16x faster than passing it by value in the above example. You'll observe a similar performance difference for all implicitly-shared objects in Qt.
Note also that in this example, you need to make 2 copies per string: once to copy it into the for-loop, and again to copy it into the addByValue() function. Pass-by-value performance degrades as the value is passed into more and more levels of nested function calls.
4. C++ developers prefer pass-by-value over pass-by-const-reference for small inputs
There are probably multiple schools of thought out there, but judging by the StackOverflow discussions I linked to above, passing-by-const-reference seems to be preferred by plenty of C++ developers.5. "You never pass a std::string object by reference, unless you want the callee to modify the caller’s string"
std::string is is definitely not a small object. Unlike QString, copying a std::string involves copying every single byte in the string. This is bad for long strings. -
Very well. I can accept that perhaps passing by const reference is still cheaper than passing by value. Perhaps the entire idea of identity objects is ridiculous, and will always be inefficient.
My argument, therefore, boils down to this: the Qt folks, in their wisdom, have chosen to use references wherever possible to maximize efficiency. This efficiency derives from the fact that the data is shared between caller and callee; in short,
A) We don't make a copy of the data.
Unfortunately, in this case, having a reference to the data is insufficient/dangerous. Therefore, to remediate the problem,
B) We immediately make a copy of the data.
This behavior is, to my mind, bizarre. An alternative choice, such as having the caller construct the object on the heap and pass ownership of it to the callee, would both make more plain what the callee is planning to do with it, and be more efficient (by avoiding any copying of the data).
Ultimately, I'd prefer that point (B) be emphasized in the documentation, simply because it seems such a strange way of doing things...
I don't know if this really merits a bug report, but I'll look into it...
-
[quote author="Copernicus" date="1405178026"]Ultimately, I'd prefer that point (B) be emphasized in the documentation, simply because it seems such a strange way of doing things...
I don't know if this really merits a bug report, but I'll look into it...[/quote]The URL is a bit misleading. It says "bugreports", but it's also the place to post ideas and suggestions.
This forum is mainly for users to discuss how to use Qt. The Qt engineers themselves don't come here much, so they're unlikely to see your suggestion. Alternatively, you could subscribe to and post to the "Interest mailing list":http://lists.qt-project.org/mailman/listinfo/interest, which is frequented by Qt engineers.
Use one of these channels to reach Qt engineers for action.
[quote author="Copernicus" date="1405178026"]
A) We don’t make a copy of the data.
Unfortunately, in this case, having a reference to the data is insufficient/dangerous. Therefore, to remediate the problem,
B) We immediately make a copy of the data.
This behavior is, to my mind, bizarre.[/quote]I'd like to understand your thoughts better. Is it bizarre because it seems pointless and contradictory to make a copy immediately after deciding not to make a copy?If so, remember that, in the case of the QNetworkRequest constructor:
- Passing-by-const-ref makes 1 copy of the QUrl (when storing the data privately)
- Passing-by-value makes 2 copies of the QUrl (once when calling the function, once when storing the data privately)
Does this reduce the bizarreness at all, or have I missed the point?
[quote author="Copernicus" date="1405178026"]An alternative choice, such as having the caller construct the object on the heap and pass ownership of it to the callee, would both make more plain what the callee is planning to do with it, and be more efficient (by avoiding any copying of the data).[/quote]Pointer semantics make code uglier though.
More importantly, how do you share a value between multiple callees?
- If you make a new allocation for each call, that becomes the same as passing-by-value, but with extra memory management steps.
- If you pass the same pointer into multiple functions, who has ownership?
** Some people use smart pointers to resolve this question, but there are ongoing debates about its appropriateness.
[quote author="Copernicus" date="1405178026"]Perhaps the entire idea of identity objects is ridiculous, and will always be inefficient.[/quote]I don't quite follow. Why is it inefficient?