Tcl Source Code

This patch implements TIP 100 - the
"unload" command for all platforms.
Changes are only in the generic directory, so it
affects all platforms.

Tested under Redhat Linux 9.0 and works
great. C extensions can be unloaded, modified & reloaded.

Example usage:

load <some_lib> <some_package>
<test the package>
unload <some_lib> <some_package>
<compile again the package>
load <some_lib> <some_package>
<test the modified package>

This can be repeated multiple times :-)

What is missing from this patch?

1) A man page for the new "unload" command.

2) A proper return value from unload. A good return value
will be whether the library has been unloaded and if
not the reference counts of usage of the library in
both trusted and safe interpreters...

George

IP - Comment Removed: 130.88.1.31

data_type - 210894

Logged In: YES 
user_id=79902

OK, it's now in the core with a fair few mods.

Changes of note:
  The test suite was substantially modified; it doesn't test
quite the same thing, but it at least doesn't print loads of
messages while working normally.
  The [unload] command has an empty return in the normal
case, since neither TIP nor documentation specified otherwise.

It would be really nice if we could have some tests that
would run properly under Windows, but I accept that this is
a major task (especially given that there were previously no
tests for [load] on that platform either.)

Logged In: YES 
user_id=79902

Hopefully it'll make 8.5a1, but perhaps not (chunky new
features just before a release are a good way to break
things, alas.)  Definitely by 8.5a2.

The splitting of unload is just a personal preference thing;
I was just thinking that doing it like that might make it
easier to expose unloading at the C level in the future. 
It's not a critical bit though.

Logged In: YES 
user_id=92283

Dear Donal,

I am a little pressed right now, so I could
not look into splitting the function.
(I think also load is written that way :-))

Is this a stopper for applying the patch?
It would be a miss to not include it in 8.5
(at least for me :-))

George

Logged In: YES 
user_id=79902

Ah, Tcl_FSLoadFile is a thin wrapper round TclLoadFile
Only the question on whether unload should be split is relevant

Logged In: YES 
user_id=79902

Code review:
Why the change from Tcl_FSLoadFile() to TclLoadFile()?
Should Tcl_UnloadObjCmd() be split into two pieces, one that
does the unloading and one that is the command?  That might
make that piece of code much easier.

Apart from that, it looks good (or at least as good as I can
tell without applying the patch.)  Upping prio

File Added - 76629: Tcl_Unload-TIP100.patch

Logged In: YES 
user_id=92283

Dear Vincent,

I ahve attached a new patch for TIP 100, which
includes everything all the previous patches contain,
along with a fix in the man page and a first implementation
of a test suite. The test suite contains the unload.test
file (loosely modelled after the load.test tests for load)
and a new addition in unix/dl (pkgua.c) which implements
a simple unloadable extension. I have tested the unload
test suite under linux (fedora core 1) without problems.
I really don't know how the suite will work under windows.
Should I have done enything? (I am confused because I
placed my C code under "unix"/dl...)

BTW, the test suite for load crashes in my system.
All other tests complete fine...

George

Logged In: YES 
user_id=32170

Upping priority, since all we need are some new tests before
this can be committed.

Logged In: YES 
user_id=32170

This looks good to me, and should work fine with vfs on all
platforms.

What it is missing, of course, are some tests --- these
should be easy to add to Tcl's existing 'load' tests, and
with a bit more effort, also to Tcl's vfs ('testreporting'
vfs) load tests.

thanks George!

File Added - 68260: TIP_100.patch

Logged In: YES 
user_id=92283

I have uploaded my latest version (#3) of the TIP 100 
patch. This patch is vfs safe and it must be applied to the core 
is TIP 100 gets voted. The previous two patches are retained 
only as a history. They should not be used in any way for 
TIP 100 implementation.

Logged In: YES 
user_id=92283

Why you say my patch is not vfs-safe?
In my understanding, the vfs has done its magic
and loaded the shared object file in memory.
What it returns me (the loadHandle) is
a token to represent this memory segment.
What I am looking is simply a memory space,
totally unrelated to the underlying file system.

What I do in my patch is what the tcl core does:
It calls the relevant vfs function to load the library,
and then checks the two initialisation functions.
Its irrelevant that I do the same thing a few clocks after.

I really cannot understand your comment. But perhaps
you know better the vfs than me (I only had a quick
look at the core sources). Can you explain in detail
how to handle it in a vfs-safe way?

George

Logged In: YES 
user_id=32170

Couple of minor comments:

+\fB\-keeplibrary\fR
+This switch will prevent the library detach from the process.

This means nothing to me at all.  Can it be explained in
non-technical jargon?

The patch is also still not vfs-safe, and the 'loadHandle'
used to find the (safe)unloadProc will be totally bogus when
run through a vfs (except in those cases where a vfs can
delete the temporary .dll/.so immediately).

File Added - 65072: TIP_100.patch

Logged In: YES 
user_id=92283

Updated patch for TIP 100.
Adds a simple version of the man page
as well as some new options to unload
(like -nocomplain, -keeplibrary)

George

Logged In: YES 
user_id=32170

>Or your point is that the symbol lookup is
>done after the vfs has loaded the file?

Yes.  Therefore no vfs changes are needed, beyond the
changes to Tcl_FSLoadFile(Ex).

As to adding things to the stubs table, just look through
the relevant stubs files (.decls) it should be pretty
obvious, else ask on tcl-core.

Logged In: YES 
user_id=92283

Yes, but doesn't Tcl_FSLoadFile calls a different
callback for different virual filesystems?
My idea of the virtual filesystem is that the
core calls Tcl_FSLoadFile. This function if
it decides that the file is in a virtual filesystem,
it calls the proper function to load the file.
Isn't this correct?

Or your point is that the symbol lookup is
done after the vfs has loaded the file?
(Perhaps this is true, as I have noticed that in the
manual the relevant callback takes a different
number of arguments. But my conclusion was that
it was a bug in the manuals :-))

If no modifications are needed in the vfs, it would be
much easier to implement it. My only problem
is that I don't know how to export a new function
through the stubs table, as I suppose the new function
would have to be exported.

George

Logged In: YES 
user_id=32170

George, what part of what I've described below don't you get?

Nothing needs to be done with existing vfs systems. There's
no need to increment the vfs version either.  No significant
aspect of Tcl's vfs internals needs to be changed.  The only
change is that a new Tcl_FSLoadFileEx needs to be provided
which looks like a more complex version of the existing
Tcl_FSLoadFile (which becomes a wrapper around the Ex
function).  Sure 'Ex' functions are ugly, but needed for
backwards compatibility, which is one of Tcl's mantras.

Logged In: YES 
user_id=92283

Changing the vfs would be a good think.
Actually this is the reason TIP 100
got so much time to be discussed.
When it was submitted we have reached a similar
conclussion. However, I don't know
how to make the needed changes.
What will be done with existing vfs systems?
We need to increament the virtual filesystem version?
I hope somebody which knows this part of tcl internals
to step in and help :-)

I aggree we should use arrays of names/pointers, as
we may need more symbols in the future.
However, adding an *Ex is not a pretty solution,
as the older function will never be used :-)

George

Logged In: YES 
user_id=32170

I have to agree with Joe -- the changes, if any, to make
this work with vfs are likely to be small, and a good TIP
should really mesh with what Tcl provides in a relatively
complete way.

Having said all that, I'm not 100% sure much extra is
actually needed.  This is because the current vfs
implementations don't actually know that a .dll has been
loaded, because we can't load directly and so instead a
copy-to-native-and-load approach is used.  On Unices where
the temp copy can be deleted immediately (line 2621 of
tclIOUtil.c) George's code will definitely work 100%.  On
Windows/Mac/Unices-which-can't-delete then we fall through
to a setup where Tcl will delete the file on exit. In this
case I believe the patch's use of TclpFindSymbol will fail
because it's not operating on a native load handle, but
rather on an 'FsDivertLoad*' structure which contains a
native load handle.

It is this case that ought, probably, to be dealt with
better by the patch.  I believe the solution is to make a
new Tcl_FSLoadFileEx which is like Tcl_FSLoadFile except it
can return 4 procs ptrs (2 for init, 2 for uninit), for 4
different symbols that it is given -- in fact for future
expandability, it might be better if it used an array of
procPtrs/symbol names and a number of elements.

The old Tcl_FSLoadFile can just wrap around this new proc
and be made available purely for backwards compatibility.

Then the Tcl command 'load' should call 'Tcl_FSLoadFileEx'
and the rest of the patch is basically the same.

hope that makes sense -- it just means there is one more
function to be adjusted in tclIOUtil.c.

Logged In: YES 
user_id=113501

Hold on a second, it's my current understanding that a TIP 
can be modified at anytime unless it's currently being voted 
on. If the VFS changes are needed (which it sounds like they 
are), we should include them now, not at some nebulous time 
in the future, so that we have a full and correct 
implementation of "unload" that fits in with the new (8.4+) file 
system code.

Logged In: YES 
user_id=92283

I thibk it does, but I haven't tested it. All the work with the
vfs is done by load. During unload its just a memory segment
that has to be freed. It will work as expected
for dlls loaded from a vfs. However, a vfs filesystem does not
get notified that the file was unloaded. This probably needs
a new vfs function and of course another TIP.

George

Logged In: YES 
user_id=32170

Out of interest, does this also work when things are loaded
from inside a vfs?

File Added - 64331: TIP_100.patch