Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash when fooling around after response->end() #17

Open
hoensr opened this issue Jun 28, 2016 · 6 comments
Open

Crash when fooling around after response->end() #17

hoensr opened this issue Jun 28, 2016 · 6 comments
Labels

Comments

@hoensr
Copy link

hoensr commented Jun 28, 2016

Hi, here's a crash I have found: When in a request handler I do stuff after calling end() on the response, e.g. have an event loop run for a second, qhttp crashes on me. I have tried both the master and dev branch.

I think it is because end() calls the done() signal, which closes the socket. Afterwards, it still receives a readyRead(), and then it crashes in QSocket::bytesAvailable(), because itcpSocket is a dangling pointer containing garbage.

I attach a minimal example. Can you reproduce the issue?
TestQhttp.zip

Here is the stack trace:
> TestQhttp.exe!qhttp::details::QSocket::bytesAvailable() Line 100 C++ TestQhttp.exe!qhttp::server::QHttpConnectionPrivate::onReadyRead() Line 81 C++ TestQhttp.exe!qhttp::server::QHttpConnectionPrivate::initTcpSocket::__l3::<lambda>() Line 118 C++ TestQhttp.exe!QtPrivate::FunctorCall<QtPrivate::IndexesList<>,QtPrivate::List<>,void,void <lambda>(void) >::call(qhttp::server::QHttpConnectionPrivate::initTcpSocket::__l3::void <lambda>(void) f, void * * arg) Line 495 C++ TestQhttp.exe!QtPrivate::Functor<void <lambda>(void),0>::call<QtPrivate::List<>,void>(qhttp::server::QHttpConnectionPrivate::initTcpSocket::__l3::void <lambda>(void) & f, void * __formal, void * * arg) Line 553 C++ TestQhttp.exe!QtPrivate::QFunctorSlotObject<void <lambda>(void),0,QtPrivate::List<>,void>::impl(int which, QtPrivate::QSlotObjectBase * this_, QObject * r, void * * a, bool * ret) Line 193 C++ Qt5Cored.dll!QtPrivate::QSlotObjectBase::call(QObject * r, void * * a) Line 124 C++ Qt5Cored.dll!QMetaObject::activate(QObject * sender, int signalOffset, int local_signal_index, void * * argv) Line 3720 C++ Qt5Cored.dll!QMetaObject::activate(QObject * sender, const QMetaObject * m, int local_signal_index, void * * argv) Line 3596 C++ Qt5Cored.dll!QIODevice::readyRead() Line 160 C++ Qt5Networkd.dll!QAbstractSocketPrivate::canReadNotification() Line 737 C++ Qt5Networkd.dll!QAbstractSocketPrivate::readNotification() Line 69 C++ Qt5Networkd.dll!QAbstractSocketEngine::readNotification() Line 153 C++ Qt5Networkd.dll!QReadNotifier::event(QEvent * e) Line 1202 C++ Qt5Cored.dll!QCoreApplicationPrivate::notify_helper(QObject * receiver, QEvent * event) Line 1150 C++ Qt5Cored.dll!doNotify(QObject * receiver, QEvent * event) Line 1090 C++ Qt5Cored.dll!QCoreApplication::notify(QObject * receiver, QEvent * event) Line 1077 C++ Qt5Cored.dll!QCoreApplication::notifyInternal2(QObject * receiver, QEvent * event) Line 1015 C++ Qt5Cored.dll!QCoreApplication::sendEvent(QObject * receiver, QEvent * event) Line 227 C++ Qt5Cored.dll!qt_internal_proc(HWND__ * hwnd, unsigned int message, unsigned __int64 wp, __int64 lp) Line 399 C++ user32.dll!0000000077289c11() Unknown user32.dll!000000007728992a() Unknown Qt5Cored.dll!QEventDispatcherWin32::processEvents(QFlags<enum QEventLoop::ProcessEventsFlag> flags) Line 829 C++ Qt5Cored.dll!QEventLoop::processEvents(QFlags<enum QEventLoop::ProcessEventsFlag> flags) Line 129 C++ Qt5Cored.dll!QEventLoop::exec(QFlags<enum QEventLoop::ProcessEventsFlag> flags) Line 204 C++ Qt5Cored.dll!QCoreApplication::exec() Line 1285 C++ TestQhttp.exe!main(int argc, char * * argv) Line 11 C++ TestQhttp.exe!__tmainCRTStartup() Line 626 C TestQhttp.exe!mainCRTStartup() Line 466 C kernel32.dll!00000000773859bd() Unknown ntdll.dll!00000000774ba2e1() Unknown

@azadkuh
Copy link
Owner

azadkuh commented Jun 28, 2016

yes, qhttp is designed to behave this way.
for such scenario you need to check qhttp::server::QHttpConnection::disconnected() signal and stop any further processing on current instances of request, response and connection.

please have a a look at issue#15

@hoensr
Copy link
Author

hoensr commented Jun 28, 2016

OK, so I have to do
connect(request->connection(), &QHttpConnection::disconnected, this, &MyListener::FoolAround);
and do my fooling around there. This works fine, thank you for clarifying this! And of course, thank you for the fast answer and for the great lib!

Oh, by the way, in order to compile the dev branch under Windows (VS 2013), I had to change some things. In httpparser.hxx line 98, I have replaced
static auto me(http_parser* p) {
by
static TImpl* me(http_parser* p) {

And in httpwriter.hxx, I have replaced

        qhttp::for_each(TBase::iheaders.constBegin(), TBase::iheaders.constEnd(),
                [this](const auto& cit) {
            const QByteArray& field = cit.key();
            const QByteArray& value = cit.value();

            this->writeHeader(field, value);
        });

by

        QHashIterator<QByteArray, QByteArray> cit(TBase::iheaders);
        while (cit.hasNext()) {
            cit.next();
            const QByteArray& field = cit.key();
            const QByteArray& value = cit.value();

            this->writeHeader(field, value);
        }

You might want to consider this to make it more platform-independent.

@azadkuh
Copy link
Owner

azadkuh commented Jun 28, 2016

humm, I do not have access to a Windows machine, i rarely have a chance to compile qhttp under Windows.
I guess vs2013 support of c++14 (minimum requirement) is not complete, and you need mingw or vs2015 (community edition just works)

@RafalNiewinski
Copy link

yea, vs2015 and mingw works fine

@hoensr
Copy link
Author

hoensr commented Jun 29, 2016

Sure, no problem. You're right, Microsoft is kind of slow in implementing the new features, e.g. the auto keyword.

I just wanted to propose that if you change these two things, it can be compiled without problem under VS2013 too, and I don't think there are any disadvantages. On the contrary, if you use QHashIterator, you can get rid of your qhttp::for_each construct.

But if you'd prefer not to, it's fine -- it's not a big deal for me to change it for myself.

@azadkuh
Copy link
Owner

azadkuh commented Jun 29, 2016

generally it's good idea to support both c++14 compatible compilers and c++11 ones. needs some #ifdefs and conditional compiling.

qhttp::for_each is analogue to std::for_each and in my taste it's more c++ rather than QHashIterator (very java style!)

it's all about taste and conventions. Hopefully QHash, QMap, ... will be supported by range for in near future and we will get rid of qhttp::for_each() and QHashIterator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants