static current session variable

[Neil Williams]

On Monday 22 August 2005 5:31 pm, Derek Atkins wrote:
> Neil,
>
> I hope you don't mind that I responded to the list -- this is
> something I think everyone should be able to see.

(And there was I thinking it was something everyone knew already!)

I don't mind at all.

> It's useful 
> information and gives us the chance to archive the information for
> posterity.  A couple years from now it might be useful to some other
> developer ;)

:-)

> Neil Williams <linux at codehelp.co.uk> writes:
> > I'm sorry if this is basic stuff, I just want to get it checked:
> >
> > Should the static current_session variable be static in qofsession.c?
>
> Techincally... no.

I understand now, thanks. "It's too much to change now but don't write any new 
code that way".

> But....  that would require changing every API everywhere in the code
> to always have a session context available. 

I appreciate how much work it would be to have a common context - it took a 
while to do that just for the merge routines.

HOWEVER, I have done exactly this in the CLI. Each CLI has it's own qof-data 
context that explicitly includes (currently) two sessions and that may even 
be increased to possibly three or even a GList of possible sessions. I 
inadvertently used qof_session_set_session in pilotqof but that is easily 
removed because the context already exists.

Unfortunately, I haven't been so circumspect in my GUI GnuCash code:
gnome/dialog-chart-export.c contains a context already in use but doesn't put 
the session into the context! Duh! That's an easy one to fix. Same with 
gnome/druid-merge.c.

I suspect that a lot of uses of current_session also have a usable context 
nearby.

> Two PROCESSES?  No.

Now this is where I wanted clarification.

> The static variable is in the data portion of the 
> library, not the text portion of the library.  The data portion is
> copy-on-write cache, so when the PROCESS makes a change, it will
> overwrite the process-specific copy-on-write version of the variable.

So each process, each application, has it's own implementation of the statics 
that are defined in the library. Yes? That explains what I saw.

> HOWEVER, it means two THREADS in the _same_ process will share the
> data.  That's why you don't want data-context to be static, because
> you might want different threads working on different data sets.

Agreed.

> > What about the static registered object hash table in qofclass.c ?
> > static GHashTable *classTable = NULL;
> > and the modules_list in qofobject.c:
> > static GList *object_modules = NULL;
>
> These are okay to remain static objects in the library for the above
> reason.

Shouldn't they eventually be made part of the same overarching context? The 
session would be a good start - each session to have it's own registered 
objects, it's own list of books, it's own set of backends and should be made 
thread-safe. After all, as far as QOF is concerned the objects are just 
another type of data.

That would need some changes in QOF to create new books within the same 
session rather than, as now, creating a whole new session. It is a big job.

> The current_session variable is the way it is for historical reasons,
> not because it is correct.  As (I hope) I've shown, the way it's done
> is wrong.

Yes, I see that and I'm very glad I implemented a context in the CLI!

> New code shouldn't use static variables for data context. 
> (Process context is okay, like those module/object lists). 

Hmm. Is that assuming that each process has to live with it's set of 
registered objects and that one process running with two incompatible sets of 
objects will not be supported?

I can't think of a use for that right now, I'm just curious.

> The 
> problem is that you cannot have two threads in the same process that
> have different sessions.  For example, wouldn't it be nice to have one
> gnucash app that has two books open at the same time in two top-level
> windows?

Yes. Can't we solve for just this problem by moving the static from QOF into a 
different Gnucash C file? Wouldn't that create two instances of the static, 
one in each gnucash application window? If they were two processes, not one 
process with two threads, wouldn't that work?

Simply reverse the line in engine/gnc-session.h:
#define gnc_get_current_session qof_session_get_current_session

Implement the qof_session_get_current_session function (which, AFAICT, is only 
implemented in GnuCash) as a gnc_get_session() function and add a deprecated 
tag to the #define?

/** \deprecated */
#define qof_session_get_current_session        gnc_get_session 
Same for gnc_set_session().

I'd then remove qof_session_get_current_session from QOF entirely.

I think I'll mark the static session variable, and the functions that use it, 
as deprecated in QOF anyway - it's clear that new code should not use it.

As far as GnuCash is concerned, the calls I've seen have all been about 
getting the current BOOK rather than the session anyway. 

return qof_session_get_book (qof_session_get_current_session ());

Getting the session has just been the way to get the book. When you don't have 
an instance to give you the book, maybe what we need is a QofBook pointer in 
something like GncMainWindow that can serve as a context? It doesn't have to 
be the same context for all.

-- 

Neil Williams
=============
http://www.data-freedom.org/
http://www.nosoftwarepatents.com/
http://www.linux.codehelp.co.uk/

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.gnucash.org/pipermail/gnucash-devel/attachments/20050822/94dfec0d/attachment.bin