File session expire when browser sends multiple concurrent requests

[amiart]

This happens when browser sends to treefrog multiple concurrent requests and each access session stored in file. One of the action threads try to write to file session, and another one reads session from the file at the same time.

In treefrog.log there is ERROR: Failed to load a session from the file store, and then session id is regenerated.

In debugger QDataStream::status() returned QDataStream::ReadPastEnd

Please add QWriteLocker in TSessionFileStore::store() just before session in written, and QReadLocker in TSessionFileStore::find() - tried this and worked for me.

Config:
Windows 10 x64
Qt 5.9 +MSVC 2015 32bit
TreeFrog 1.17.0

application.ini:
Session.StoreType=file
Session.AutoIdRegeneration=false
Session.LifeTime=0
Session.CookiePath=/
Session.GcProbability=100
Session.GcMaxLifeTime=1800

[amiart]

Maybe a better solution would be something like this (instead of QReadWriteLock):
https://github.com/qtproject/qt-solutions/tree/master/qtlockedfile

[treefrogframework]

@amiart, Thanks for detailed reporting.
Fixed in 4143fa7.
Could you test it?

[amiart]

Thank you, but do we really need 2 locks (one for processes and one for threads) ?
Can't we just have one file lock (something like QtLockedFile) which will block both theads and processes ?

In current implementation the rwLock is global static variable, so: QWriteLocker locker(&rwLock); // lock for threads will block requests for ALL sessions (file lock would block only one session).

[treefrogframework]

2 locks are needed.

QWriteLocker locker(&rwLock); // lock for threads will block requests for ALL sessions (file lock would block only one session).

It's my compromise.
If you mind the performance really, you can use the session store for Redis.

[amiart]

OK, one more question, why tf_lockfile() doesn't have implementation for Windows ?

[treefrogframework]

It seems open() function locks against opening by other processes on Qt/Windows, so the function includes locking of the file.
Could you re-test including them?

[treefrogframework]

App server (tadpole.exe) of TreeFrog is multiprocessing and multithreading.

application.ini:

# Number of application server processes to be started.
MPM.thread.MaxAppServers=4

# Maximum number of action threads allowed to start simultaneously
# per server process.
MPM.thread.MaxThreadsPerAppServer=128

[amiart]

Session still expires, but not as often as before.

I think QReadLocker/QWriteLocker should be placed before file is open.

BTW: You mention than 2 locks are needed. I suppose you mean WinAPI LockFile that blocks only processes. But take a look at source code of QLockedFile. They don't use LockFile, but named mutexes.

Here is example:
https://msdn.microsoft.com/en-us/library/windows/desktop/ms686927(v=vs.85).aspx

[treefrogframework]

Thanks!
I could not reproduce the bug. I think you are correct.
Fixed in 3045609.
Could you retest it?

Try this setting in application.ini;

 SystemLog.Layout="%d %5P (%i) [%t] %m%n"

[amiart]

Session still expires. It's because object destruction order is wrong.
Currently objects are destroyed in the following order:

~QWriteLocker() <- releases lock
~QFile() <- flushes data to file

If we move QReadLocker/QWriteLocker before QFile like this:

QWriteLocker locker(&rwLock); // lock for threads
QFile file(sessionDirPath() + session.id());

the destruction order would be OK:

~QFile() <- flushes data to file
~QWriteLocker() <- releases lock

[treefrogframework]

@amiart, OK.
Maybe fixed in 9877444.

[amiart]

Finally fixed, thank you very much for help !

[treefrogframework]

Thank you, too.