search

weloytty / MSMQ.Messaging

A drop in replacement for System.Messaging for use with .NET Core (on Windows only)
MIT License
38 stars 14 forks source link

InvalidCastException when sending MSMQ message #5

Open kwende opened 2 years ago

kwende commented 2 years ago

We are using your library under .net core and have discovered that, at least it seems, when the code is under load (~10 req/sec) we get the following exception:

image

I'm not entirely sure what all the code in here is doing, but it appears to be constructing COM property arrays (presumably for placing/serializing into MSMQ). Maybe? I haven't dealt with MSMQ at this level before. Regardless, a particular function of yours has me curious that perhaps we're running into some issues with a collision of intent of an array:

image

When looking at the code that's crashing, I see you're casing it and thereby treating it as a uint at the time of the crash. However, the object in the array at that index is a GCHandle (which seems to be the second intent of the handles array). Maybe the fact you're using this array to store different things is causing an issue? Or perhaps index collisions?

Without knowing all of the specifics I modified your code to be more mindful of nullity and the underlying types. It seems to have prevented the crash we're experiencing. But I'm opening this PR to open a discussion and not to necessarily recommend this code as-is. I just don't understand what's going on underneath to say for sure if this is even a valid fix.

One last thing: when I cleaned up the code to be mindful of whether the underying type as a GCHandle, I had to also be aware to not call .Free() on a null object. It seems as though the GCHandle, if it exists, was being freed and collected previously. I tried to determine if there was anything async going on (threads stomping on each other), by putting things inside lock statements, but to not avail.

weloytty commented 2 years ago

This raises an interesting question... what to do about flaws in the original System.Messaging code? I've not tried 'improving' things, this is more or less a straight copy of Microsoft's reference source. The main work I spent on this was lobbying Microsoft to change the license on the source so I could create a .NET Standard version that could be used for dotnet core.

So I am very hesitant to make changes (even if they, as in this case, make things better) just because I'm worried about backward compatibility.

So I have to think on this for a bit -- maybe create a "this is the reference source" branch and a "hey, this makes things better" branch. The big thing I would want would be xplat, but that's not really in the cards due to the reliance on mqrt.dll.

kwende commented 2 years ago

So, I cannot speak to the specifics of your concerns with Microsoft because I'm not in the middle of all of that, however from a "user" perspective (a.k.a. someone just wanting to use something that works), having a special .net standard / core / next gen branch is perfectly fine. Currently our back is against the wall because we're moving to .net core, but some aspects of our product relies on MSMQ.

Bear in mind I am still not entirely sure as though I understand all that's going on under the hood in that code. So, your comment about my changes making things better might be premature as I could have opened up other issues. However, we did internally verify that the root issue is calling these methods in a heavily threaded environment. By enqueuing and sending messages to your layer serially the issues evaporated.

If you do decide to make a new branch, it's not impossible we may wind up sending several PRs your way in the next few months if we encounter more.