Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Segfault on SteamGameServerNetworkingSockets.SendMessages #421

Closed
IvMisticos opened this issue May 22, 2021 · 13 comments
Closed

Segfault on SteamGameServerNetworkingSockets.SendMessages #421

IvMisticos opened this issue May 22, 2021 · 13 comments

Comments

@IvMisticos
Copy link

IvMisticos commented May 22, 2021

When calling SteamGameServerNetworkingSockets.SendMessages the Segmentation fault occurs in the following code:

// Fields
byte[] Data;
HSteamNetConnection Connection;
int SendFlags;

void Send() {
// The send method
var message = Marshal.PtrToStructure<SteamNetworkingMessage_t>(SteamGameServerNetworkingUtils.AllocateMessage(Data.Length));

message.m_conn = Connection;
message.m_nFlags = SendFlags;

Marshal.Copy(Data, 0, message.m_pData, Data.Length);

var messages = new[] {message};

var messagesr = new long[messages.Length]; // Just so that it exists, idk what to do with it
SteamGameServerNetworkingSockets.SendMessages(messages.Length, messages, messagesr);
}

What could go wrong, what am I doing wrong? Same happens when I use SteamGameServerNetworkingSockets.SendMessageToConnection.

@IvMisticos
Copy link
Author

Is this an issue with my C# code, or I misunderstand the Steamworks documentation, or an issue with Steamworks itself?

@yaakov-h
Copy link

I suspect it is both, this looks extremely tricky.

The immediate point of suspicion is that PtrToStructure does not map that memory to a .NET struct, but copies it.

Then, and I might be reading the method signatures wrong here, but SendMessages appears to want a pointer to a set of pointers, but Steamworks.NET is instead passing it a pointer (marshalled array) to a set of structs.

It also looks like you're not setting some of the fields on SteamNetworkingMessage_t - such as the function to free the data, which looks particularly sus.

Lastly, since the data is not pinned or in any way retained for the native Steamworks components, the .NET memory manager is free to move the data around in memory and garbage-collect it while Steam is still trying to use it, which is bound to crash eventually.

@IvMisticos
Copy link
Author

The free function is managed by Steamworks itself if I specify amount of bytes in SteamGameServerNetworkingUtils.AllocateMessage() method arguments, pretty sure I don't have to unless I specify 0 or such.

@IvMisticos
Copy link
Author

I'm copying the data to the buffer Steam allocated and does manage and will free on its own with Marshal.Copy (at least that's what I think it should do), means .NET should not free the data that is.. not even managed, it's Steamworks allocated memory, unmanaged. I just copy my data to a pointer Steamworks gives me.

@IvMisticos
Copy link
Author

IvMisticos commented May 25, 2021

But PtrToStructure might be why, I'll look into it. What should I use instead? Just casting the pointer to the needed type?

That clearly didn't work, lol Cannot declare a pointer to a managed type 'Steamworks.SteamNetworkingMessage_t'

@IvMisticos
Copy link
Author

@rlabrecque and I do agree that it looks weird that the Steamworks documentation requires pointers, but we are passing an array of structs.

@yaakov-h
Copy link

I’m not sure if your game engine supports it or not, but Unsafe.AsRef is probably more correct here than PtrToStructure. Unless you want to copy it back with StructureToPtr?

@IvMisticos
Copy link
Author

Alright, used Unsafe.AsRef, but don't think it helped still. Might consider passing array of pointers with custom Steamworks.NET build to see if it works.

@rlabrecque
Copy link
Owner

#411

This may help you, I can't quite take a look at this deeper yet though

@IvMisticos
Copy link
Author

So far NOT calling message.Release helped us move a bit further, still testing.

@rlabrecque
Copy link
Owner

Hmm can you check this one out too: #388

@IvMisticos
Copy link
Author

This is due to Release indeed. Can we have it fixed so that it passes what it actually should, or make it accept an argument?

@IvMisticos
Copy link
Author

Also please expose m_pfnFreeData since it's supposed to be settable by users, unless you're willing to make a pooling and stuff helper.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants