Dalija Prasnikar

1 hour ago, havrlisan said:

You're right, I see it now. Are there any downsides to that approach? And if not, is there a reason Embarcadero did not do that in initialization and finalization sections?

Only problem you can face is if you are using dynamically loaded packages as the RTTI for that package will point to invalid memory. But I am not sure how relevant is that to the macOS as I don't know if you can even use runtime packages there.

1 hour ago, havrlisan said:

Here's the link to the QP issue: https://quality.embarcadero.com/browse/RSP-44207

Thanks! I added the test project.

11 hours ago, Joseph MItzen said:

Like a lot of stuff at this company, things are probably done a certain way just because they've always been done a certain way by the same people who have worked there since the 80s or 90s.

AFAIK, Idera runs the whole IT infrastructure for multiple companies under their umbrella.

10 minutes ago, havrlisan said:

The first stack goes through System.Classes:BeginGlobalLoading, which directly calls EnsurePoolToken and is not avoidable.

Yes, it will call EnsurePoolToken, but if the global token already exists it will not lock PoolLock while it tries to create one.

2 hours ago, havrlisan said:

That is what I did, but I still want to hear your thoughts on this, and whether this is a bug that should be reported, or not.

Yes, this looks like a classic deadlocking bug. One thread locks one lock, while another locks other and they are dead in the water. It should be reported.

Possible workaround would be acquiring context at the application initialization (with KeepContext) and releasing it on shutdown. That way you would prevent EnsurePoolToken called from KeepContext to lock PoolLock. 

https://blogs.embarcadero.com/we-are-experiencing-a-hardware-outage/

16 minutes ago, Vandrovnik said:

Nobody - but they don't say anything.

This is why I said that this was not a software problem, but a hardware problem. Beyond that anything anyone says is pure speculation.

4 minutes ago, Attila Kovacs said:

Ok, CM_RELEASE has no job in form's wnd proc, I have to hop to TWinControls WndProc

It is directly dispatched in TCustomForm through procedure CMRelease(var Message: TMessage); message CM_RELEASE; 

1 minute ago, Attila Kovacs said:

@Dalija Prasnikar

Ok, I see now where the problem is. However, since VisualManager_AcceptMessage is not virtual, the only way to fix that without adding a patched Vcl.Forms to the project is to handle CM_RELEASE without calling the WndProc...

As I always use a common form, this should be manageable. I just have to examine what CM_RELEASE should do... let's see.

CM_RELEASE is simple

procedure TCustomForm.CMRelease;
begin
  Free;
end;

Yes, you can intercept it in your form.

Just now, David Heffernan said:

If I'm running a company like Emba, then I'm not putting production servers on physical hardware with single points of failure.

Who said that it is server that failed?

When workers cut my phone cable I was without access to Internet it for three days (that was long before broadband Internet was available). There is just some stuff where you cannot possibly have redundancy, or the cost would just be a way too much. It is not like they are running mission critical infrastructure, so they need to be prepared for absolutely every scenario. And even then, there are things that might happen outside of their (or anyone's) control.

Not everything that ever happens is result of some incompetence.

As far as Facebook is concerned you can read it here https://en.wikipedia.org/wiki/2021_Facebook_outage What this page does not cover is breaking into the server room, but that was something one of people involved posted on Twitter at the time. The problem was that entering the server room also required authentication which was not working because their whole network infrastructure was down. I have no idea what they have or have not learned from that incident. 

Error happens when CM_RELEASE is sent to a form. The fix is to avoid calling VisualManager in that scenario. Instead in WndProc, you can also add check in VisualManager_AcceptMessage method before the code within tries to access FVisualManagerInitialized field.

procedure TCustomForm.WndProc(var Message: TMessage);
  ...

  inherited WndProc(Message);

  if (Message.Msg <> CM_RELEASE) and VisualManager_AcceptMessage(Message) then
    VisualManager_WndProc(Message);
end;

I am sure that everyone here has everything in duplicate, so when something dies they can replace it in an instance with minimal interruptions, especially if it dies over the weekend during the huge cold spell. Stuff happens, no matter how small or big the company is. Do I need to remind you that on one instance Facebook folks had to literally break into their server room because they got locked out of it. 

I am not going to defend the lack of official communication, this is something that needs to change, but this is also not something that people on lower hierarchy levels (meaning the ones that blog and communicate with us) can do on their own.

FWIW, you can forget about software jokes, it is piece of hardware that died.

6 minutes ago, Attila Kovacs said:

404 😉

Yeah... it will be replaced by another official statement. But, I guess in the meantime, the title tells part of the story.

23 minutes ago, PeterPanettone said:

Thanks. When do you estimate the problems will be resolved?

https://blogs.embarcadero.com/we-are-experiencing-a-hardware-outage/

4 minutes ago, Kas Ob. said:

Thread.Execute is finished and exited,

Which will happen during the cleanup in TThread destructor because of fEven.SetEvent which is called in TerminatedSet. Please read everything I posted before. If you don't trust me, then run the code yourself. We are running in circles now.

3 minutes ago, Kas Ob. said:

for example, the thread in question above should have TerminatedSet called in the destructor before the inherited Destroy this will make sure the thread is not blocking on an event that might not be triggered or will take long time to trigger, this is equal or the same as freeing the event (before the the inherited) and forget about it.

The TThread destructor will call all that. I suggest you look at the source code. After that inherited destructor call is finished, the background thread will no longer run and it will be safe to call fEvent.Free. 

4 minutes ago, Kas Ob. said:

Undefined behavior ?!!

I said "was possible race condition", which is fixed if you call inherited thread destructor before you call fEvent.Free.

9 minutes ago, Kas Ob. said:

TThread.Free in my opinion should never ever called, all and every TThread by design should clean up after themselves, well that is my humble opinion, the whole point of leaving a class/creature meant to run only once such as thread which can effecting the whole process, should be considered as falling knife, draw the path/loop or what ever then let it run its course cleaning and exiting gracefully.

Self destructing threads, while useful, have their own issues. They cannot be terminated directly, waited for, or properly cleaned up during the application shutdown and can cause various issues and exceptions during the shutdown. Basically, you lose proper application cleanup. 

Actually, you can do proper cleanup with such threads, too, but only if you add plenty of code that will also have to fiddle around with Windows API, and when doing all that, you are effectively rewriting infrastructure that is already in place inside TThread class. And then you are back to the square one, where you need to handle thread cleanup in proper order, basically achieving nothing comparing to using thread class that you need to manually Free - which again will work properly, if you pay little attention.

3 minutes ago, Kas Ob. said:

Well, yes, but waiting on event that will not be triggered because the thread is trying to call the inherited destructor, for me is undefined behavior, will if finish or will it not and when ?

Event will be triggered when TerminatedSet is called, which will be called as part of the thread cleanup process triggered by inherited destructor. There is no undefined behavior. Thread will finish, there was only a possible race condition where fEvent could have been released while background thread was still using it.

4 minutes ago, Kas Ob. said:

If TThread destructor being called while the thread is running

Default TThread destructor will wait for thread to finish, so there will be no undefined behavior. That is why this inherited destructor needs to run first. Otherwise, you must always wait for a thread before you call Free on it. Because it is easy to forget calling WaitFor before calling Free every time you use a thread somewhere, it is much safer to call the inherited destructor first, because that is something you need to write only once.

@Remy Lebeau fEvent should be released after inherited destructor runs as thread might still be running at that time if thread is not waited for before calling Free (which is not common pattern in Delphi code) 

destructor TTimerThread.Destroy; 
begin 
  inherited Destroy; 
  fEvent.Free; 
end;

12 minutes ago, Rollo62 said:

I think, to refine a wrong, too early decision later on can be a lot harder than necessary.

Depends on the decision. Replacing Sleep with event later on is trivial. 

In this case we don't even know whether we need to have a thread that will sleep or wait on an even in the first place. This is the "wrong direction" I am talking about.

4 minutes ago, Anders Melander said:

While waiting on an event is much more complicated than just calling sleep, if one is to use threads one might as well learn how to do these things. Threads are difficult and pretending they aren't will just lead to lessons learned, the hard way, later on.

If you know absolutely nothing about threads, then Sleep can be a first step towards solution. It will at least give you a simple proof of concept code. If you need to learn everything at once it can be overwhelming.

But, again, at this point we are discussing the Sleep vs events while we don't even know if we are going in the right direction.

10 minutes ago, Lars Fosdal said:

There are many better alternatives to Sleep. 

I don't think we are at that level yet. We first even have to establish what is even remotely appropriate solution, given that the problem is vague. And then we can start optimizing the code. So Sleep() suggestion is a valid one. If Sleep could be used, then we can offer better solutions than using Sleep().

2 minutes ago, dormky said:

That's not a problem here, the code called by the timer is "fire and forget".

It seems like you don't even need a timer in a background thread, you just want to periodically run some task in the background thread. 

procedure TForm2.Timer1OnTimer(Sender: TObject);
begin
  TTask.Run(
    procedure
    begin
      // call here anything that needs to run in the background thread
    end;
end;

Again, it would be so much easier to give you some suggestions if you would just show some of your code.