Skip to content
This repository has been archived by the owner on Aug 22, 2021. It is now read-only.

confused me:why use an empty job for sync in listener_accept_func #100

Open
578141611 opened this issue May 30, 2020 · 4 comments
Open

confused me:why use an empty job for sync in listener_accept_func #100

578141611 opened this issue May 30, 2020 · 4 comments

Comments

@578141611
Copy link

`
err_t listener_accept_func (void *arg, struct tcp_pcb *newpcb, err_t err)
{

...
SYNC_DECL
SYNC_FROMHERE
...

DEAD_ENTER(client->dead_aborted)
SYNC_COMMIT
DEAD_LEAVE2(client->dead_aborted)

// Return ERR_ABRT if and only if tcp_abort was called from this callback.
return (DEAD_KILLED > 0) ? ERR_ABRT : ERR_OK;

`
I can not understand why you need to handle ERR_ABRT?
In my opinion,there is only one thread in tun2socks,then we occpied the thread in function "listener_accept_func".
can you answer me. thank you. @ambrop72

@ambrop72
Copy link
Owner

Hi,
lwIP needs us to return ERR_ABRT from the listener callback if we have tcp_abort'ed the client. If we are not very careful to do that, lwIP could crash or misbehave.
DEAD_* is a mechanism to allow us to determine if a certain event has occurred in a certain time frame during processing. In this case, if DEAD_KILL_WITH is called in client_abort_pcb within SYNC_COMMIT (which directly runs pending job handlers), then DEAD_KILLED will be > 0. The whole thing is kind of a hack but is necessary.

@ambrop72
Copy link
Owner

ambrop72 commented May 30, 2020

On the other hand I suppose the code could be changed to just do the minimum necessary in listener_accept_func, instead of running event loop processing, i.e. no SYNC_COMMIT and related stuff, always return ERR_OK. But it would not be optimal since we would delay sending/receiving that can be done right, so lwIP might do something suboptimal because of that. Probably not a big issue though.

@ambrop72
Copy link
Owner

ambrop72 commented May 30, 2020

Ah, I think I remember, generally the pattern is that within a lwIP callback I directly run event loop jobs that have resulted from whatever actions were just started (e.g. adding a packet to a buffer), to make sure that all data processing that can be done is done right away - this is what the SYNC_COMMIT does. Without this, if lwIP gives us another packet right afterward, we might not be able to place it in the buffer at all, due to the way that the data processing pipeline works - the processing from the previous packet might be pending on the job queue still, so not ready to handle another packet.

@578141611
Copy link
Author

Thank you. @ambrop72

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants