Sqlite_orm: Is the library thread-safe?

[sivabudh]

Eg. multiple readers / writers are trying to access the same storage at the same time. Does the library arbitrate the accesses or I need to mutex it.

[sivabudh]

...because right now, I think I'm getting a SQLITE_BUSY (the database file is locked) which is throwing a std::system_error exception.

Is there some settings to enable multi-threaded access?

[sivabudh]

From this article,

https://www.sqlite.org/threadsafe.html

It says that the default mode is serialized. I'm running the application on Windows installed via vcpkg. So, I take it that SQLite is already in serialized mode?

[sivabudh]

The way I structured my code is that I made the class methods static so that any threads can access the database. Is that the best practice way?

Here's an example.

// SomeDb.h
class SomeDb
{
public:
   struct Record
   {
         int id;
         std::string some_data;
         boo resolved;
   };
   static Record search_by(int id_);
};

and here's the implementation file

// SomeDb.cpp
auto make_sqlite_storage()
{
   using namespace sqlite_orm;
   auto storage = make_storage("somedb.sqlite",  make_table("Record", 
                                       make_column(...)  
                             );
   storage.sync_schema();
   return storage;
}

// !! Important !! using a single object holding connection to Sqlite
auto g_Storage = make_sqlite_storage();

Record SomeDb::search_by(int id_)
{
    using namespace sqlite_orm;
    auto record = g_Storage.get_all<Record>(where(c(&Record::id) == id_));
    return record[0]; // assumes record always found for simplicity 
}
void SomeDb::insert(std::string new_data_)
{
    using namespace sqlite_orm;
    auto record = g_Storage.insert<Record>(
        Record{-1, new_data_, false}
    );
}

Then, Thread A can access it like this:
SomeDb::insert("some new data");

while at the same time Thread B should be able to read:

   auto foundRecord = SomeDb::search_by(3);

[sivabudh]

Or, should I make g_Storage a private member variable of SomeDb class instead and take out the static keyword from the methods?

[fnc12]

1) static has nothing common with initial issue about thread safety cause you can both access your storage via static functions and via non-static member functions by passed reference to a storage from different threads. IMHO static should me omitted if it can be omitted cause it makes architecture more clear and mockable
2) if you want to access storage from different threads you should call open_forever right after making a storage cause storage can create different connections as a result of data races inside a storage
3) the only thing related to thread safety is busy_timeout member function in storage_t class. There is no inner mutex in storage and there is nothing about handling inner sqlite multithreading. So I'd advice you using your own mutex before accessing a database

But if you're interested we can investigate multithreading in sqlite. First of all I can add threadsafe function. You'll be able to check whether or not your storage is threadsafe.

[sivabudh]

Thanks. I think to have the threadsafe function to check storage is threadsafe is very helpful.

[sivabudh]

Supposing there were 2 threads (one writer, one reader) that need to access the same SQLite database file, when you designed this library, did you intend for different threads to share the same storage reference or for each thread to hold its own storage "copy"?

[sivabudh]

I never knew about open_forever. May I conclude that if an application was multithreaded, that is a SQLite database file will be accessed by different threads simultaneously, the best practice is to _always_ call open_forever?

[sivabudh]

You may also be interested in this StackOverflow post:

http://charlesleifer.com/blog/multi-threaded-sqlite-without-the-operationalerrors/

and this one:

https://stackoverflow.com/questions/10325683/can-i-read-and-write-to-a-sqlite-database-concurrently-from-multiple-connections

[sivabudh]

How would I enable WAL with sqlite_orm?

https://blog.xojo.com/2015/12/21/speed-up-sqlite-with-write-ahead-logging/

[fnc12]

open_forever guarantees that any access to the database through a storage will be performed with one and only one connection

[fnc12]

Also I can add PRAGMA journal_mode easily with storage.pragma.journal_mode = ... interface for WAL

[sivabudh]

OK, RE: open_forever, then instead of needing to use my own mutex, I could just call open_forever and all accesses will be serialized by sqlite_orm.

In other words, open_forever serializes all accesses for me.

So do you recommend me using open_forever if I'm running into std::system_error due to SQLITE_BUSY or should I use WAL?

[sivabudh]

By the way,

I will be upgrading to 1.2 and trying out the new getters/setters feature. Will feedback if I run into any problems. =)

[fnc12]

you still will receive SQLITE_BUSY sometimes after open_forever cause you can write something while something other is being written by other thread. open_forever is required to just eliminate data races inside storage class itself, not sqlite database. Without calling open_forever one can create and set two different connections to storage_t::current_transaction member. This will lead to undefined behavior. Also you can increase busy timeout right now.
Also I'll add threadsafe member function to storage_t class and push it to a new branch. Also I'll add pragma journal_mode in that branch

[sivabudh]

Thanks, after considerations, I will put mutex around all accesses to SQLite.

FWIW, I will use a writer / reader mutex for better performance. See: http://doc.qt.io/qt-5/qreadwritelock.html

[sivabudh]

And thanks for adding threadsafe and journal_mode, I will be testing them via vcpkg when you make the release. (1.3?) =)

[fnc12]

It would be better if you test threadsafe and journal_mode before release right from their special branch or dev branch cause I need a feedback before merging a feature. And these two features are not enough for a new release

[sivabudh]

OK. I will test for you. Just let me know when it's pushed to Dev. Thank you.

[fnc12]

sorry I got sick and didn't finish these functions yet. I'll make it in a week

[fnc12]

@sivabudh hi. I've finally finished a branch with threadsafe and journal_mode features. You can inspect it in https://github.com/fnc12/sqlite_orm/pull/165 . Also feel free to review it and/or comment it. Also you can pull a branch named threadsafe-and-journal-mode to test journal mode and threadsafe. BTW threadsafe is available already now by calling sqlite3_threadsafe();. This function takes no arguments, lol. I've created an interface for it: int sqlite_orm::threadsafe().
Journal mode can be accessed with pragma member in storage: storage.pragma.journal_mode(journal_mode::OFF);

[fnc12]

@sivabudh what's your threadsafe function result?

[fnc12]

@sivabudh I'm gonna merge #165 and close the issue

[fnc12]

merged #165