Threading question

[David Schwartz 413]

48 minutes ago, Kas Ob. said:

@David Schwartz Can you confirm if my suggestion above ( using UniqueString ) solve those AV?

12 minutes ago, David Schwartz said:

I'll give it a try and let you know.

Well, there's a problem right off the bat. 

 

procedure StatusOut(const stat: string);
var
  AMsg:string;
begin
  AMsg := UniqueString(stat);

'stat' is a const arg, and UniqueString is looking for a 'var' arg.

procedure UniqueString(var str: UnicodeString); overload;

 

Perhaps the 'const' argument is part of the problem here?

[Guest]

10 minutes ago, David Schwartz said:

Well, there's a problem right off the bat. 

replace UniqueString assigning line with these:

  SetLength(AMsg, Length(stat));
  Move(stat[1], AMsg[1], Length(Msg) * SizeOf(Char));

 

[David Heffernan 2291]

26 minutes ago, Remy Lebeau said:

You mean WM_COPYDATA, not WM_COPY.

Yes, sorry 

[David Heffernan 2291]

14 minutes ago, David Schwartz said:

Well, there's a problem right off the bat. 

 

procedure StatusOut(const stat: string);
var
  AMsg:string;
begin
  AMsg := UniqueString(stat);

'stat' is a const arg, and UniqueString is looking for a 'var' arg.

procedure UniqueString(var str: UnicodeString); overload;

 

Perhaps the 'const' argument is part of the problem here?

A big part of your problem is trying stuff at random without any understanding of the reason why. You say that you find windows a swampy mess, but the code you presented here can be reasoned with quite easily if one has the knowledge, and it isn't that advanced. Unfortunately quite aot of the "advice" you have received has been incorrect. 

 

The only way for you to make progress is to stop guessing and trying random suggestions from people who don't understand the area. Get a stack trace with madExcept. It takes little time to add that. 

[David Schwartz 413]

4 minutes ago, Kas Ob. said:

replace UniqueString assigning line with these:

  SetLength(AMsg, Length(stat));
  Move(stat[1], AMsg[1], Length(Msg) * SizeOf(Char));

 

It seems to me that if the string is gone at the time this method is called, then copying it is only going to trigger the same AV fault, right? If it's not, then it's unnecessary.

[Remy Lebeau 1257]

20 minutes ago, David Schwartz said:

Well, there's a problem right off the bat. 

 

procedure StatusOut(const stat: string);
var
  AMsg:string;
begin
  AMsg := UniqueString(stat);

'stat' is a const arg, and UniqueString is looking for a 'var' arg.

procedure UniqueString(var str: UnicodeString); overload;

 

Perhaps the 'const' argument is part of the problem here?

The correct way to use UniqueString() in this example would be like this:

procedure StatusOut(const stat: string);
var
  AMsg: string;
begin
  AMsg := stat;
  UniqueString(AMsg);
  // use AMsg as needed...
end;

 

[Remy Lebeau 1257]

13 minutes ago, Kas Ob. said:

replace UniqueString assigning line with these:

  SetLength(AMsg, Length(stat));
  Move(stat[1], AMsg[1], Length(Msg) * SizeOf(Char));

 

If you are going to do that, it would be simpler to use SetString() instead:

SetString(AMsg, PChar(stat), Length(stat));

Or Copy():

AMsg := Copy(stat, 1, MaxInt);

Especially since both of these approaches handle empty strings, whereas accessing index 1 of an empty string is technically undefined behavior, even though Move() won't do actually anything when the Length is 0.

Edited April 20, 2020 by Remy Lebeau

[Anders Melander 1634]

1 hour ago, David Schwartz said:

I'd like to think I have a better-than-average understanding of multi-threading.

No disrespect but the same is true of many of those that have below average expertise - The Dunning–Kruger effect.

 

1 hour ago, David Schwartz said:

In contrast, I find Windows to be more of a swampy mess when it comes to multi-threading.

I disagree but it doesn't really matter; The principles are the same regardless of the platform.

 

Under any circumstances, at this point, your (and our) time would be better spent if you tried to locate the source of the problem instead of having everybody guess.

 

1 hour ago, David Schwartz said:

The fact that you guys keep arguing with each other about things that should be quite simple only points to the underlying complexity of how Windows offers up it's ugly, hydra-like threading models.

Don't blame Windows for that. If you don't understand it then maybe leave threading to those that do.

As far as I can see the majority of this thread is people restarting discussion about issues that have already been covered, people suggestion things to try and you trying it with no idea if or why it might or might not work.

[David Schwartz 413]

I know there are a couple of very long and detailed books that have been written that amount to "The Tao of Multi-Threading in Windows (and Delphi)". I'll leave it to others to master this topic since I have not really needed to do so up to this point in my career. My experience with multi-threading is with kernels that take perhaps 10 pages to explain the entire model. They're very simple, succinct, consistent, and designed to support the needs of (soft) real-time interrupt-driven programming.

 

I cut my teeth on the "Manager vs. Monitor" debates that flared up in the 1980s as they applied to multi-processing, and I did a lot with both tightly-coupled and loosely-coupled multi-processing. Then DOS showed up and people mostly laughed. Then Windows showed up and people gagged.

 

Need I mention some of the high-profile failures Windows NT caused during the 90's when it was used to control real-time process-control applications? (Does anybody remember the fiasco around the baggage handling system at Denver International Airport that delayed its opening by over a year? The popular press blamed the DBMS they were using, but I used the same DBMS and it worked flawlessly; the guys I talked with there said it was how Win NT was so inconsistent in how it processed things that made it keep locking up.) I think the numerous episodes like this speak quite eloquently for themselves.

 

Several people have pointed out that the original developers of this code may have also lacked a depth of understanding based on what I've shown here. From comments in the code, I'd say the threading was added around 2004 and it does not appear to have been significantly altered since then, except for a few odd bits here and there given the comments that were left.

 

The threading model that this app uses is implemented in several units that form the core of a couple dozen other apps, and I'll get skewered if I make any changes that affect them. My task is simply to migrate this thing from Win XP to Win 10, and there are issues showing up in Win 10 that may have been around forever but weren't manifesting in XP.

 

All I know is that with minimal calls to the tracing & logging methods, everything works fine. When I send more and more tracing data out, I hit a point where I'm getting AVs in unpredictable areas.

 

I can tell there are some race conditions going on. I just can't tell exactly where they're happening or why, and how to either fix them or circumvent them.

 

I'm trying different things people have suggested, but to little avail so far. I do appreciate the support and suggestions, tho.

Edited April 20, 2020 by David Schwartz

[Vincent Parrett 657]

Not sure how much scope you have to change things, but this is what I use for internal messaging from threads https://github.com/VSoftTechnologies/VSoft.Messaging - it uses custom messages rather than trying to pass data using wparam. 

[aehimself 388]

Holy mess, I skipped only one day with my gadget addiction and I came back to see my E-mail inbox exploded! As it was mentioned countless times, you must get a stack trace of where the error happens. Without that most probably we are all looking at the wrong place. Far from ideal, but SendMessage works in this scenario. I doubt that Win7 - Win10 will be an issue either; I am testing most of my code with everything from Windows 2000 and up and so far only new features (which does not exist in earlier editions) caused headache. And just because it's a framework, don't be afraid to touch it! At work we have a similar setup - part of our legacy app's code is used as a base for many other applications. I started to sort out some basic leaks and you can not imagine what was buried deep below. A framework is the nothing but code, which can contain bugs even if thousands of other applications are using it.

If you are absolutely sure you can not get a stack trace, at least pollute your methods with debug messages using a new logger with proper synchronization. When the AV occures, just look which method started and did not finish. Alas, using proper synchronization in your new debug logger might solve the synchronization issue (if it is a synchronization issue after all...) with those tiny-tiny delays. So yeah, reinventing the wheel is not a smart thing to do, if we have MadExcept and similar.

[David Schwartz 413]

I've been trying to install MadExcept and not having much luck so far. We have severe security restrictions to deal with in our environment. In particular, our normal logins do not have Admin privs. We have a separate login "a-<username>" that DOES have Admin privs, but it's only temporary. It's kinda like sudo but more restrictive.

 

The install is putting the data in to the Admin's registry, not the normal user's registry. So it's there if I run Delphi as Admin but not otherwise.

 

When i try to add the BPLs to Delphi with my normal login, I get an error saying I don't have permission to read the files. Not even the Help files!

 

We can't change ownership, and are restricted in terms of granting wider visibility to files. We're even blocked from running RegEdit.

 

Getting a slow response from IT Dept isn't helping.

 

Grrrr....

Edited April 21, 2020 by David Schwartz

[David Schwartz 413]

9 hours ago, Vincent Parrett said:

Not sure how much scope you have to change things, but this is what I use for internal messaging from threads https://github.com/VSoftTechnologies/VSoft.Messaging - it uses custom messages rather than trying to pass data using wparam. 

I cannot change the infrastructure, per se, but I can change how this app works for its own needs. The means of sending data between the thread and the form is of no consequence outside the app. The form does not communicate with the thread once the thread is initiated. The thread does, however, communicate with some supporting services and I can't change that part. But as far as I can tell, they're working fine.

 

Just one question ... mostly what I need to pass is strings, 25-250 characters in length, mostly single lines under 80 chars. Using this mechanism, would I need to pass them as arrays of bytes rather than as pointers? (I think this is where my current problem lies in that the pointers to the objects or strings being sent are getting zapped.)

 

Since these are Records, are they passed by value (ie, copied)? If so, would it make sense to break things up into, say, 127-char blocks and send them that way?

Edited April 21, 2020 by David Schwartz

[Guest]

11 hours ago, David Schwartz said:

It seems to me that if the string is gone at the time this method is called, then copying it is only going to trigger the same AV fault, right? If it's not, then it's unnecessary.

The string is not gone yet here, and the pointer is valid to be used, that was my point from start.

The pointer/string that been passed up to this point is not guaranteed to be safe until reaching "exit" of this procedure, because it involve sending it to other output, and that was my point from start, but in other words.

Yes i still think those lines will solve your problem, and a confirmation would be nice !, don't expect an AV here, but if you tried to catch exception here and this get caught, then the problem is earlier stage at sender thread.

 

My usage of UniqueString is wrong and that is my mistake, i am sorry for this, i got confused with between Delphi RTL and a version of that function in my library, my bad ,sorry.

 

Quote

Yes, because it CAN'T switch to PostMessage().  That would grant the caller permission to free the string data before it has actually been used to update the Memo.  The Add() must be synchronous, regardless of its implementation, blocking the caller until the update is complete to ensure the integrity of the string data.

My mistake here too, keep repeating the mess between SendMessage and PostMessage, my bad again, while SendMessage as you described is guaranteed and that is the normal behaviour and right one too, but are there virtual controls that save the items in list ? ( in this case will save that pointer/string to local ) and just update the count or just redraw, such design is there and used.

 

When i discussed SendMessage in first place i mentioned all consequent operations that involve send this pointer forward, the developer who wrote the thread in first place should guarantee the safe delivery of the data, and the message procedure that receive the data does belongs to him to build as he see fit, his mistake to assume the data will be safe for any process comes after that, i think this what happened another developer came and changed the way data been stored to a file or replaced the visual element responsible to show it.

[Vincent Parrett 657]

1 hour ago, David Schwartz said:

Just one question ... mostly what I need to pass is strings, 25-250 characters in length, mostly single lines under 80 chars. Using this mechanism, would I need to pass them as arrays of bytes rather than as pointers?

No, just as string fields in the record.

 

1 hour ago, David Schwartz said:

Since these are Records, are they passed by value (ie, copied)? If so, would it make sense to break things up into, say, 127-char blocks and send them that way?

Yes by value, they are wrapped in a class while in the queue, see TVSMessageWrapper<T>. You don't need to anything fancy, just create a message record type with the fields you want to pass around. We use have been using this library (or variations of if, I refactored it a few years ago) in FinalBuilder for a long time to send messages from the stepping engine to the UI and the logger and I've never seen any issues with message contents etc. There's a sample application (vcl and fmx versions) which shows it passing strings around just fine.  
 

[David Schwartz 413]

@Kas Ob. it does not seem to make a difference. An AV is still happening. Maybe from something else.

[David Schwartz 413]

11 hours ago, Vincent Parrett said:

Not sure how much scope you have to change things, but this is what I use for internal messaging from threads https://github.com/VSoftTechnologies/VSoft.Messaging - it uses custom messages rather than trying to pass data using wparam. 

The  VSoft.Messaging.Channel unit is referring to VSoft.WeakReference (containing TWeakReferencedObject) in its uses clause. That file is not in the package.

[Vincent Parrett 657]

3 minutes ago, David Schwartz said:

The  VSoft.Messaging.Channel unit is referring to VSoft.WeakReference (containing TWeakReferencedObject) in its uses clause. That file is not in the package.

That is in a separate repo - https://github.com/VSoftTechnologies/VSoft.WeakReferences

 

 

[David Heffernan 2291]

8 minutes ago, David Schwartz said:

@Kas Ob. it does not seem to make a difference. An AV is still happening. Maybe from something else.

The code in the original post that uses SendMessage to send a string is correct. Your problem is elsewhere. A madExcept bug report at the point of the exception will tell you more. 

[David Schwartz 413]

Just now, David Heffernan said:

The code in the original post that uses SendMessage to send a string is correct. Your problem is elsewhere. A madExcept bug report at the point of the exception will tell you more. 

I'm blocked waiting on help from our IT Dept getting madExcept installed. I'm having file permissions issues, and I don't have permissions to fix them either.

 

I realize that it's necessary at times for devs to have access to production networks to test their work, but giving us unrestricted access carries with it some pretty severe security protocols that make you wonder who they're trying to keep out exactly. I've never worked anywhere that imposes such draconian restrictions on developers. It makes work extremely cumbersome and often impossible without IT's intervention. I'm sure IT isn't happy with it either.

 

[David Schwartz 413]

Well, I still haven't been able to install MadExcept, but I did figure out that one problem was a constructor where I neglected to call 'inherited'. It didn't get fully initialized and was the source of one of the AVs. 

 

Another one is where I seem to be calling a method on a freed object. Haven't tracked that one down yet. Probably an object allocated via an Interface.