Don't require a connection id to use mg_wakeup

[jameshilliard]

Makes broadcasts cleaner I think.

See: #2786

[scaprile]

Sorry, this breaks the very essence of having this function in the first place.
https://mongoose.ws/documentation/tutorials/core/multi-threaded/

[jameshilliard]

Sorry, this breaks the very essence of having this function in the first place.

Maybe I'm misunderstanding but I thought the purpose of this function is to pass data to the mongoose thread from a separate thread. I'm not sure how this would break anything.

[cpq]

@jameshilliard you can send wakeup to the listening connection, if you don't have any specific connection to wake up.

[jameshilliard]

you can send wakeup to the listening connection, if you don't have any specific connection to wake up.

I mean, you technically can but it seems to be a lot of excess complexity vs something along these lines, sending to the listing connection for a broadcast like this just seems to be weird to me. You can also see in the multithreading example how this is also a pretty significant reduction in lines of code and cleaner api for broadcast type operations.

[cpq]

@jameshilliard if you want to just wake up the manager without sending specific data to a target connection, we could make it by accepting 0 as a target connection id:

mongoose/src/sock.c

Line 681 in 8a8ff2a

if (mgr->pipe != MG_INVALID_SOCKET && conn_id > 0) {

In that condition, just remove "conn_id > 0" part.
Would that give what you're looking for?

[jameshilliard]

if you want to just wake up the manager without sending specific data to a target connection

Well I do need to send data, just like in the change I made here to the multi-threaded example.

In that condition, just remove "conn_id > 0" part.
Would that give what you're looking for?

Wouldn't that result in no event being fired due to the connection filtering in wufn?

When testing I had to change this as well for sending an event to a manager without a specific target connection:

mongoose/src/sock.c

Line 642 in 8a8ff2a

if (t->id == *id) {

[cpq]

@jameshilliard I guess we have different ideas about the usage of it.

Your example is about broadcasting.

Our example about targeting a single connection - imagine that requests to a certain API endpoint require a long calculation, and API endpoint takes some parameters. So every connection starts a thread with some unique parameters - that's why we had struct thread_data which you deleted, cause for broadcasting, it does not matter. And a worker thread generates a response that is unique, and is required for only one connection.

So yeah, we're talking about different use cases.

By accepting 0 as a connection ID, we can do both targeted wakeup, and broadcasts. Yes, we would need to change wufn() too. And no, we can't modify the example code as you suggest, cause it showcases targeted wakeup, not broadcasts.

[scaprile]

Sorry to interrupt, the second part of our example (one-to-many), IMHO, does just what the OP wants, maybe not the way he'd like to.
https://mongoose.ws/documentation/tutorials/core/multi-threaded/#the-one-to-many-case
https://github.com/cesanta/mongoose/tree/master/tutorials/core/multi-threaded-12m

[jameshilliard]

no, we can't modify the example code as you suggest

I don't see what the issue is here, I simply changed the example to demonstrate how the changes I made here are an improvement for broadcast operations, I could separate the example changes by creating a new example if that's preferred.

it showcases targeted wakeup, not broadcasts.

Eh, it's clearly a broadcast example, my changes in this PR significantly simplify these sort of operations as you can see by the fairly significant reduction in overall lines of code of the example by eliminating the pointless(for broadcast operations like this) parent connection id state tracking.

Sorry to interrupt, the second part of our example (one-to-many), IMHO, does just what the OP wants, maybe not the way he'd like to.

Right, the second example is effectively doing the same thing I am trying to do. The issue I have is that the example is unnecessarily complex/hacky due to the API limitations. The unnecessary connection id tracking requirement of the current API becomes particularly problematic if you are wanting to listen on multiple ports from a single manager as well I think. It also seems to effectively make it a requirement that one start the thread calling mg_wakeup from within the event handler which shouldn't actually be necessary as you can see in my changes to the example.

[scaprile]

Well, I happen to have written that second example, and don't feel the same way about it... 😉

[jameshilliard]

don't feel the same way about it... 😉

I've been trying to understand why but I seem to be missing the reasoning behind requiring a connection id for cases when one is obviously not wanted/needed(i.e. broadcasts).

[scaprile]

Maybe because everything in Mongoose is a connection. You may not want to get a response, but Mongoose needs to identify the endpoint.

[jameshilliard]

Maybe because everything in Mongoose is a connection.

I mean, this isn't really changing that pattern from what I can tell, it's simply special casing connection id 0 to send the mg_wakeup event to all connections instead of a specific connection.

You may not want to get a response, but Mongoose needs to identify the endpoint.

But it doesn't actually need to identify a specific connection/endpoint for doing a broadcast(as the example changes I made show, this requirement to identify a specific endpoint doesn't really exist in practice), since the actual endpoint one wants to send to is in reality all endpoints/connections.