search

Bauble / bauble.classic

this is how Bauble and Ghini both started
GNU General Public License v2.0
10 stars 34 forks source link

clean up threading code #253

Closed mfrasca closed 8 years ago

mfrasca commented 8 years ago

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.

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 8 years ago

This issue was moved to Ghini/ghini.desktop#133