Hi,

Clicking on Tools/Rescan FPC sources crashes the IDE. However, this is not the case if the IDE includes only its basic packages.

Can anybody reproduce?

Stephano

2011/3/25 patspiper :

I don't know if it is relationed, but here happens that, at random, when the auto-completion menu should appear (e.g. ctrl+space) lazarus will go on access violation with the popup: "ok to kill / cancel to ignore" (or vice versa). But ignoring is still possible to save, close and reopen lazarus without problems.

Kjow

Can you find out, what package is the trouble maker?

Mattias

It turned out to be a package I had converted from Delphi and installed in the IDE. It has System.IsMultiThread:=true in the initialization section. Commenting this out eliminated the crash.

What does this property do, and what are the effects of commenting it out? The cross platform package uses threads and apps that use it must use -dUseCThreads (in Linux).

TBuildManager.RescanCompilerDefines clear caches
TFPCTargetConfigCache.NeedsUpdate compiler file changed "/home/me/Programs/fpc/2.4.2/bin/fpc" FileAge=1301012632 StoredAge=0 TFPCTargetConfigCache.Update /home/me/Programs/fpc/2.4.2/bin/fpc TargetOS=linux TargetCPU=i386 CompilerOptions=
ExtraOptions=-Fr/home/me/Programs/lazarus/lazarus-svn/components/codetools/fpc.errore.msg PATH=/home/me/Programs/fpc/crossbinutils/bin:/home/me/Programs/fpc/2.4.2/bin:/home/me/Programs/fpc/2.4.2/lib/fpc/2.4.2:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games RunTool /home/me/Programs/fpc/2.4.2/bin/fpc -iTOTP -Pi386 -Tlinux -Fr/home/me/Programs/lazarus/lazarus-svn/components/codetools/fpc.errore.msg RunTool /home/me/Programs/fpc/2.4.2/bin/fpc -va -Pi386 -Tlinux -Fr/home/me/Programs/lazarus/lazarus-svn/components/codetools/fpc.errore.msg compilertest.pas
TFPCTargetConfigCache.Update: has changed
WARNING: TLCLComponent.Destroy with LCLRefCount>0. Hint: Maybe the component is processing an event?
[TMainIDE.Destroy] A
[FORMS.PP] ExceptionOccurred
Sender=EAccessViolation
Exception=Access violation
Stack trace:
$087B0D52 TIDEPROGRESSWINDOW__COUNT, line 288 of progresswnd.pas TApplication.HandleException Access violation
Stack trace:
$087B0D52 TIDEPROGRESSWINDOW__COUNT, line 288 of progresswnd.pas

Stephano

It is set by the RTL, when switching from single threaded to multi threaded and back. Only read this, do not set it.

What does this property do, and what are the effects of commenting it out? The cross platform package uses threads and apps that use it must use -dUseCThreads (in Linux).

Mattias

Just curious, but what does a "IDE installed package" and "rescanning the FPC source code tree" have to do with each other? That seems like some IDE dependencies that shouldn't exist in the first place?

The scan uses multi threading, which was broken by the installed package.

Mattias

Mattias Gaertner wrote

The scan uses multi threading, which was broken by the installed package.

I was reading the thread and replying before I read the whole thread. Indeed that installed package did do something it was not supposed to do.

In all fairness, how are all developers supposed to know that? Isn't there a better way that FPC could hide such functionality (not allowing a developer to set a RTL controlled variable they are not supposed do). That should really be filed as a bug against FPC. They [fpc] should rather use some singleton class with a read-only property, or simply a function (function IsRTLMultiThreaded: boolean;) or something, not just a global read-write variable - that's just sloppy!

You know the answer, Graeme...

But I agree that this could be done better. Keep the IsMultiThread variable for Delphi compatibility, but use a private variable for the real stuff and set the Delphi compatible one together with the real one internally.

Regards,
Sven

If you don't know the purpose of a variable, why change it? The documentation and comment do not say that you should set it.

AFAIK when writing your own thread manager you must set it.

Mattias

Mattias Gaertner wrote

If you don't know the purpose of a variable, why change it?

Well you know programmers, especially 'newbie' ones... They sometimes do just as dumb things as end-users. ;-)

The documentation and comment do not say that you should set it.

I haven't checked the documentation, but does it also say you shouldn't set it. :)

AFAIK when writing your own thread manager you must set it.

Still no excuse for a global variable. Simply registering that thread manager with some RTL class could automatically return True if the function IsRTLMultiThreaded(): boolean is called. Global variables are renowned for being "unsafe" and so 80's style of developing... improved alternative do exist now.

And if Delphi jumps off a cliff so will all FPC developers? Well, I will NOT.

While we can't help for the Delphi developers being stupid, we could try and learn from their mistakes and make FPC better! I'm pretty sure many here would agree, that we all choose to use FPC with the hopes that it is BETTER than Delphi, not just a damn second rated clone (always being one step behind Delphi).

There are many new designs and improved programming styles or design patterns that reduce (or totally eliminate) the need for such unsafe global variable usage.

[fpc] should rather use some singleton class with a read-only property,

A class is not required, FPC supports unit-level properties as well :-)

DoDi

Hans-Peter Diettrich wrote

A class is not required, FPC supports unit-level properties as well :-)

True, though that is a language feature I have never seen used by anybody in the wild. :)

Yes, but this is no reason to make the RTL child safe. It's a tool not a toy.

> The documentation and comment do not say that you should set it. I haven't checked the documentation, but does it also say you shouldn't set it. :)

I think the better question is: Has he read some documentation? And if yes, which one make him believe to set it.

lol.
Changing a time critical variable to a function just to prevent vandalism by newbies. For a moment I really thought you were serious. :)

Mattias

I have to partially agree with Graeme on this point, especially that this package was ported from Delphi. The symptoms were on the surface unrelated to the cause. I had to debug for quite a while until I stumbled upon the culprit IsMultiThread assignment buried deep within 10-15 units. A compile time 'read only' error would have saved me a lot of grief.

Stephano

The joke was to use a function.
A read only property would be nice and a separate procedure to set it. But I don't know if this is compatible. A function is not compatible too.

Mattias

I was inprecise when I mentioned 'this point'. I was alluding to the very early posts in this thread, and in particular the read only issue. My bad.

A read only property would be nice and a separate procedure to set it. But I don't know if this is compatible.

Too bad. It could save others a lot of frustration.

Stephano

Mattias Gaertner wrote

The joke was to use a function.

And my point was that anything (not necessarily a function) is better that a global read-write variable that could cause something as serious an an application crash.

A read only property would be nice and a separate procedure to set it.

Exactly what I was trying to get across.

But I don't know if this is compatible.

Compatible with what? In the event that somebody implements a new Thread Manager? And how often does that really happen? If you mean being able to query the RTL to see if Multii-threading is enabled, then simply being able to read a property or function that returns a boolean is perfectly compatible with a boolean global variable.

patspiper wrote

A compile time 'read only' error would have saved me a lot of grief.

And not to mention that debugging global variables are notoriously hard. Yes MSEide supports the ability to notify the developer if something like a global variable changes, but I believe Lazarus IDE doesn't.

I used it several times, just to trap and debug writing into global variables.

DoDi

Sure, it is not like there are useful things to do in the FPC project :-)

The more you hold the users hand, the sloppier they get.

Answering from the same experience as with TP's death:

We will probably jump of the cliff, and climb back on 5-8 years later. By then most Delphi code will be dead or maintenance only, and compatibility will be less important.

The trouble is that you always tend to focus on the tiniest little detail that you ran into the last 10 minutes, and then try to make that the center of the FPC universe, as if it were a leaking nuclear reactor. Everything is a balance.

While in reality it is more the missing of a ribbon or foil around a matchbox that avoids the matchbox opening accidentally (*)

(*) Do you see matchboxes with ribbons in reality? No, because not all
theoretical holes are plugged because that it

There are many new designs and improved programming styles or design patterns that reduce (or totally eliminate) the need for such unsafe global variable usage.

Since when is information hiding and access control new? It's eighties technology.

Keep in mind that design patterns are not new. They go back to 1996, and even then they only claimed to standarize practices already in use for decades.

Not always. Roughly it is the same difference as between a field and a property. A field/variable allows to take the address (and thus pass to VAR parameters), properties (class or global) and functions don't.

Still, I think this is much ado about nothing.

They apparantly you don't use unices. Since errno and cerrno are such beasts.

True, but the issue with them is: you cannot use them to create a fool proof read-only property without using a function because there is no unit level encapsulation aka private/protected on unit level: the variable read by the property cannot be in the implementation part of the unit because it wouldn't be accessible at declaration time of the property.

Making IsMultiThreaded a function is a no-no: it is used in really speed critical code paths and using an inline function neither because it's used mostly in assembler code in the rtl like:

if ismultithreaded then
<do things locked>
else
<do things without locking>

Marco van de Voort wrote

We will probably jump of the cliff.....

Wow, you are really sad!

The trouble is that you always tend to focus on the tiniest little detail

And setting a value of a global variable allows an application to crash... Oh that's nothing serious. Just goes to show how fragile the FPC code base is... a rather scary thought.

While in reality it is more the missing of a ribbon or foil around a matchbox that avoids the matchbox opening accidentally (*)

Please stick to programming, you suck as a philosopher.

ACK

Making IsMultiThreaded a function is a no-no: it is used in really speed critical code paths and using an inline function neither because it's used mostly in assembler code

Simple solution: the variable continues to exist, under a different name. The property only implements Delphi compatibility :-)

DoDi

Possible, but I fear this is also only half of a solution: a property cannot replace a variable, see var parameter problem or assembler code.

What about the following solution:

* add a variable in the implementation section of system.pp that will be the real workhorse for "should the RTL be threadsafe" (basically what IsMultiThread is now)
* when setting that private variable also set the "old" IsMultiThread variable (but never read it inside the RTL)
* provide a setter function for the private "should the RTL be threadsafe" variable and document it with a big warning that it is only for use inside a thread manager.

Regards,
Sven

And how can other code check it fast?

Mattias

Now I get what Florian ment with the "time critical" part... In that case it's indeed not a good solution. Sorry for the noise.

Regards,
Sven

In such cases the user gets a compiler error message, so that he can know what has to be changed in the ported code.

DoDi

Personally I think such methodology implemented on a broad scale (e.g. all globals) will cause more havoc then it will solve.