search

Ghini / ghini.desktop

plant collections manager (desktop version)
http://ghini.github.io/
GNU General Public License v2.0
24 stars 14 forks source link

clean up threading code #133

Open mfrasca opened 8 years ago

mfrasca commented 8 years ago

From @mfrasca on January 2, 2016 12:17

The older parts of Bauble use fibra "a sophisticated scheduler for cooperative tasks". That project does not exist any more where it used to be hosted, actually its home page had been taken by an other project which has been archived too. The author @simonwittber copied it to https://github.com/simonwittber/fibra five years ago, but the code is frozen at version 0.0.17.

now I'm writing parts which all make use of the standard threading module, which is easier for me to read and more consistent with standard practices.

one problem with threading is in the use we make of SingletonThreadPool. It works fine with fibra because fibra is not multithreaded. (IIUC)

the SingletonThreadPool will call .close() on arbitrary connections that exist beyond the size setting of pool_size, e.g. if more unique thread identities than what pool_size states are used. This cleanup is non-deterministic and not sensitive to whether or not the connections linked to those thread identities are currently in use.

I tried to do NullPool, but the impact was major, and nothing in the test cases would work any more. And I definitely do not want to run tests differently than the code itself.

A first action related to this issue is to fine tune SingletonThreadPool: Considering the above, I guess we can still use this policy if we can guarantee that we will never run more threads than the size of the pool.

Copied from original issue: Bauble/bauble.classic#253

mfrasca commented 8 years ago

I think I was looking for this information, in particular the part related to from contextlib import contextmanager.

mfrasca commented 8 years ago

I added prio:high because I'm more and more relying on multi-threaded code and I fear the software might become less stable.

mfrasca commented 8 years ago

part of this issue is making really sure that sessions that get opened are also closed, but to do so we need some definitive documentation about session lifetime in different parts of the interface.

the following are considerations and they might contain mistakes.

the SearchView owns one session which stays open as long as Bauble is open. the SearchView uses its session in READ mode. when the SearchView activates an object editor, the same session is passed to the editor and the editor will upgrade the use to WRITE mode. this might cause trouble if not done carefully. I think it would be best to explicitly start a 'would write' transaction when opening an editor and getting the session from the object received. keep in mind that when you use SQLite, locks are on the whole database, while with a real DBMS they can be at record or table level. threads that READ from the database start sessions and stay in the background. starting the editor from the 'insert' menu at the moment does not pass the SearchView session to the editor and I think it should, at least for consistency.

mfrasca commented 6 years ago

Using the scoped_session magic seems to most trouble to the moment we're closing the program (i nodi al pettine). We really need to stop a moment and have a serious look at the interaction between threads, sessions, and database objects. I would use as model the recently added picture importer, but I'm open to suggestions, criticism, and learning.

I hope someone the image label will do some magic too.

upon program termination, we get a fairly long list of: ProgrammingError: SQLite objects created in a thread can only be used in that same thread.The object was created in thread id x and this is thread id y