|Title:||TIP 100 Reference Implementation - Unload|
|Submitter:||petasis||Created on:||2003-10-14 14:36:36|
|Status:||Closed||Last Modified:||2009-07-29 19:39:57|
|Closed on:||2004-02-24 23:13:36|
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
dkf added on 2009-07-29 19:39:57:
IP - Comment Removed: 18.104.22.168
dkf added on 2008-11-02 18:27:48:
data_type - 210894
dkf added on 2004-02-25 06:13:36:
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.)
dkf added on 2004-02-24 17:08:32:
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.
petasis added on 2004-02-24 00:11:54:
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
dkf added on 2004-02-21 21:18:12:
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
dkf added on 2004-02-21 20:17:49:
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
petasis added on 2004-02-14 03:48:21:
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
vincentdarley added on 2004-01-09 23:11:51:
Logged In: YES user_id=32170 Upping priority, since all we need are some new tests before this can be committed.
vincentdarley added on 2003-11-24 17:10:15:
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!
petasis added on 2003-11-22 06:25:07:
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.
petasis added on 2003-10-23 03:48:54:
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
vincentdarley added on 2003-10-23 00:32:50:
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).
petasis added on 2003-10-22 21:10:13:
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
vincentdarley added on 2003-10-15 21:54:40:
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.
petasis added on 2003-10-15 17:13:36:
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
vincentdarley added on 2003-10-15 16:58:02:
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.
petasis added on 2003-10-15 16:45:34:
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
vincentdarley added on 2003-10-14 22:51:10:
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.
mistachkin added on 2003-10-14 22:33:10:
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.
petasis added on 2003-10-14 22:25:41:
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
vincentdarley added on 2003-10-14 22:07:51:
Logged In: YES user_id=32170 Out of interest, does this also work when things are loaded from inside a vfs?
petasis added on 2003-10-14 21:36:36:
File Added - 64331: TIP_100.patch