This one is caused by an initialization bug of some sort, the byteSizes of ODBC-structs are incorrect. In short, the solution is to do: ExternalStructure allSubclassesDo: [:each | each compileFields] after you load a package which defines ExternalStructure subclasses (like ODBC does). For some reason, in an image after loading: ExternalStrucure >> byteSize "Return the size in bytes of this structure." ^self compiledSpec first bitAnd: FFIStructSizeMask SQLHENV compiledSpec first: 65536 FFIStructSizeMask: 65535... So basically, when you allocate the struct a handle of a ByteArray of size byteSize, you'll sooner or later you access unallocated memory and crash. Recomputing all the compiledSpecs with the doit above leads to the correct sizes (4 in the case of SQLHENV) being stored in the compiledSpecs. Either way, not raising an error when the size for a struct when and'ed with the mask, but instead causing a silent crash later on just seems bad bad bad to me.

On Oct 20, 2009, at 4:16 37PM, Mariano Martinez Peck wrote:

On Tue, Oct 20, 2009 at 8:06 AM, Henrik Johansen <henrik.s.johansen@veloxit.no

...

wrote: This one is caused by an initialization bug of some sort, the byteSizes of ODBC-structs are incorrect.

In short, the solution is to do: ExternalStructure allSubclassesDo: [:each | each compileFields] after you load a package which defines ExternalStructure subclasses (like ODBC does).

Hi Henrik! Nice to see you arround ;)

In SqueakDBX I use FFI, I have subclasses of ExternalStructure and I don't remember having to do that. The only thing I do in those subclasses is to implement a class side method initialize like this:

initialize super initialize. self defineFields

And then, if you see defineFields you will se that at the end, it does a compileField... You can see defineFields in FFI documentation: http://wiki.squeak.org/squeak/2426

So, in my opinion a better solution may be this. What do you think ?

Cheers

Mariano

Errr, so in conclusion, you didn't do exactly what I did, but something else which results in doing exactly the same, but in addition define accessors for the fields. So you did have to do that, even though you don't remember it. :P Here's the reasons why I feel the solution you found, by itself, does not constitute a good solution: - If you DON'T remember to do it, the image of people loading your project might crash silently. - Neither FFI itself nor ODBC has remembered to do so (it's done in the loadFFI script in ScriptLoader) - Debugging it if you haven't read some arbitrary wiki-page was a real pain (partly because it doesn't crash as soon as the error is made) - Even you, who had actually read the wikipage, didn't make the association to this issue. A couple of alternative approaches: a) - Returning an error if a basicSize request would return 0 informing that fields have not been properly initialized (Or silently doing a recompile) b) Wait for MC1.6's atomic loader so you can be sure the fields method is compiled when a new external structure is installed :) c) Don't store into compiledSpec if fields is empty, on the presumption it will not be used until fields has been defined. (This won't actually work, as loading from MC will only execute class initialization only if class initialize method is defined in package being loaded (store works the same), but: d) - Do a compileFields: call in ExternalStructure>>initialize, and instead create a createAccessors method which can be overridden if necessary. ) a) and c) will both suffer from the issue that if you load/merge a newer version with additional fields defined, you get the same flaky behaviour as is experienced now. b) is probably some time off in the future, depending on when someone gets the time to implement trait support. a), but without the silent recompile and a helpful error message might be the best short-term option, since it'd remind authors to add class initialize-methods, and tell a user what to do if they've forgotten to do so instead of crashing the image. I've attached a changeset implementing c) to the issue. Cheers, Henry

Correction, I reported the problem, but anyone trying to use ODBC on Pharo will have a bad day until we fix something. Whether or not a bug lives in Pharo is of less interest to potential users (and to us!) than is having working code. Bill ________________________________ From: pharo-project-bounces@lists.gforge.inria.fr [mailto:pharo-project-bounces@lists.gforge.inria.fr] On Behalf Of Mariano Martinez Peck Sent: Friday, October 23, 2009 8:21 AM To: Pharo-project@lists.gforge.inria.fr Subject: Re: [Pharo-project] #1339 - ODBC crashes vm 2009/10/22 Henrik Johansen <henrik.s.johansen@veloxit.no<mailto:henrik.s.johansen@veloxit.no>> On Oct 20, 2009, at 4:16 37PM, Mariano Martinez Peck wrote: On Tue, Oct 20, 2009 at 8:06 AM, Henrik Johansen <henrik.s.johansen@veloxit.no<mailto:henrik.s.johansen@veloxit.no>> wrote: This one is caused by an initialization bug of some sort, the byteSizes of ODBC-structs are incorrect. In short, the solution is to do: ExternalStructure allSubclassesDo: [:each | each compileFields] after you load a package which defines ExternalStructure subclasses (like ODBC does). Hi Henrik! Nice to see you arround ;) In SqueakDBX I use FFI, I have subclasses of ExternalStructure and I don't remember having to do that. The only thing I do in those subclasses is to implement a class side method initialize like this: initialize super initialize. self defineFields And then, if you see defineFields you will se that at the end, it does a compileField... You can see defineFields in FFI documentation: http://wiki.squeak.org/squeak/2426 So, in my opinion a better solution may be this. What do you think ? Cheers Mariano Errr, so in conclusion, you didn't do exactly what I did, but something else which results in doing exactly the same, but in addition define accessors for the fields. So you did have to do that, even though you don't remember it. :P I think there there was a misunderstood. The one who has the problem was Schwab,Wilhelm Here's the reasons why I feel the solution you found, by itself, does not constitute a good solution: - If you DON'T remember to do it, the image of people loading your project might crash silently. - Neither FFI itself nor ODBC has remembered to do so (it's done in the loadFFI script in ScriptLoader) - Debugging it if you haven't read some arbitrary wiki-page was a real pain (partly because it doesn't crash as soon as the error is made) - Even you, who had actually read the wikipage, didn't make the association to this issue. I dont understand. In SqueakDBX (not ODBC) I have never had to send the message defineFields. It is sent by the class side initialize. So, I don't forget it. It is automatically. When you download SqueakDBX from Monticello, that's is done. A couple of alternative approaches: a) - Returning an error if a basicSize request would return 0 informing that fields have not been properly initialized (Or silently doing a recompile) b) Wait for MC1.6's atomic loader so you can be sure the fields method is compiled when a new external structure is installed :) c) Don't store into compiledSpec if fields is empty, on the presumption it will not be used until fields has been defined. (This won't actually work, as loading from MC will only execute class initialization only if class initialize method is defined in package being loaded (store works the same), but: d) - Do a compileFields: call in ExternalStructure>>initialize, and instead create a createAccessors method which can be overridden if necessary. ) a) and c) will both suffer from the issue that if you load/merge a newer version with additional fields defined, you get the same flaky behaviour as is experienced now. b) is probably some time off in the future, depending on when someone gets the time to implement trait support. a), but without the silent recompile and a helpful error message might be the best short-term option, since it'd remind authors to add class initialize-methods, and tell a user what to do if they've forgotten to do so instead of crashing the image. I've attached a changeset implementing c) to the issue. You forgot to upload the changeset ;) Cheers, Henry _______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr<mailto:Pharo-project@lists.gforge.inria.fr> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project