Skip to content

Race condition causes segfault in librustuv when using libgreen with stream reading #15231

New issue

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

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

Already on GitHub? Sign in to your account

Closed
retep998 opened this issue Jun 28, 2014 · 8 comments · Fixed by #15252
Closed

Race condition causes segfault in librustuv when using libgreen with stream reading #15231

retep998 opened this issue Jun 28, 2014 · 8 comments · Fixed by #15252

Comments

@retep998
Copy link
Member

When using libgreen on Windows (verified not to occur with libnative) when I'm reading from a TcpStream and the remote host closes the stream (specifically I'm closing the client by pressing ctrl + c), the server silently terminates with no error messages. It appears SIGSEGV is being called.

The code I was writing which caused this issue can be found here: https://github.com/NoLifeDev/NoLifePony/tree/c26be99492403c734718d37ed91d0546ec190785

Using Windows 8.1 x64 with latest Rust nightly.

Attempts to backtrace using gdb have failed: https://gist.github.com/retep998/e820aea8f019b29660fe

@huonw huonw added A-iOS and removed A-iOS labels Jun 28, 2014
@huonw
Copy link
Member

huonw commented Jun 28, 2014

cc @alexcrichton

@retep998
Copy link
Member Author

I have acquired a backtrace by attaching gdb to a running process rather than running the process from gdb to begin with (why this works I have no idea)

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 5832.0x1738]
0x004ccc9c in stream::read_cb::h0aa53bd142f22e38jZf::v0.11.0.pre ()
(gdb) backtrace
#0  0x004ccc9c in stream::read_cb::h0aa53bd142f22e38jZf::v0.11.0.pre ()
#1  0x004db9ac in uv_process_tcp_read_req (loop=0x8010c8, handle=0x80ae88, req=0x80aebc) at src/win/tcp.c:885
#2  0x004d121e in uv_process_reqs (loop=0x8010c8) at src/win/req-inl.h:152
#3  0x004d1b92 in uv_run (loop=0x8010c8, mode=UV_RUN_DEFAULT) at src/win/core.c:345
#4  0x0047d6ca in uvio::UvEventLoop.EventLoop::run::h1d15421f9e024ae3r1a::v0.11.0.pre ()
#5  0x00452765 in sched::Scheduler::run::ha79c3ff2c3f823d5SWa::v0.11.0.pre ()
#6  0x00451a61 in sched::Scheduler::bootstrap::h85fe42e0b2715604gSa::v0.11.0.pre ()
#7  0x004707a0 in SchedPool::new::closure.6620 ()
#8  0x0047070b in thread::Thread$LT$$LP$$RP$$GT$::start_stack::closure.6611 ()
#9  0x0059a4ff in thread::thread_start::h5ba3e4810fa11878Ead::v0.11.0.pre ()
#10 0x74d5495d in KERNEL32!BaseThreadInitThunk () from C:\WINDOWS\SYSTEM32\kernel32.dll
#11 0x76fb98ee in ntdll!RtlInitializeExceptionChain () from C:\WINDOWS\SYSTEM32\ntdll.dll
#12 0x76fb98c4 in ntdll!RtlInitializeExceptionChain () from C:\WINDOWS\SYSTEM32\ntdll.dll
#13 0x00000000 in ?? ()

@retep998
Copy link
Member Author

After debugging with visual studio I figured out the exact line causing the segfault. At https://github.com/rust-lang/rust/blob/master/src/librustuv/stream.rs#L253 it is rcx.result = nread;. rcx here is a null reference, which was transmuted from getting the data of the handle, which is apparently 0. I don't know what is setting it to 0 though.

@retep998
Copy link
Member Author

It appears even sending packets to the connection causes it to die. Has anyone even tested libgreen on Windows with TCP?

@retep998
Copy link
Member Author

I have determined the cause of the bug to be a race condition. Specifically every time I open up a connection, StreamWatcher::new() is called 3 times and StreamWater::read() is called once. If the 3 new streams all occur before the read call, the handle->data pointer remains non-zero. If one of them occurs after the read call, then the handle->data pointer becomes zero, and the read callback segfaults. I have verified this through multiple runs and uverrln! statements everywhere.

@retep998
Copy link
Member Author

To further expand, StreamWater::new() is called when a TcpStream is created, and also everytime I clone the TcpStream. Since cloning a TcpStream is the only way to share it across tasks (it doesn't impl Share), I had to spawn two tasks, one to read and one to write. Because I spawned the reading one first it would start a read before I cloned the tcp for the writing task, and once I cloned it, the data pointer got reset so the data pointer the read callback received was null thereby causing the segfault.

@retep998 retep998 changed the title Program terminates when a TcpStream is closed by remote host when using libgreen on Windows Program terminates when a TcpStream is closed by remote host when using libgreen Jun 28, 2014
@retep998 retep998 changed the title Program terminates when a TcpStream is closed by remote host when using libgreen Race condition causes segfault in librustuv when using libgreen with stream reading Jun 28, 2014
@alexcrichton
Copy link
Member

Has anyone even tested libgreen on Windows with TCP?

Yes, no commit gets pushed to master unless it passes all tests on windows, and that includes many networking tests.

Do you have a small test case which reproduces this bug? It's likely that there's just some different callback-invoking behavior on windows than on unix that was accidentally not dealt with.

@retep998
Copy link
Member Author

Here is a minimal code example: https://gist.github.com/retep998/224bfb130448e086df76

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jun 29, 2014
When cloning a stream, the data is already guaranteed to be in a consistent
state, so there's no need to perform a zeroing. This prevents segfaults as seen
in rust-lang#15231

Closes rust-lang#15231
bors added a commit that referenced this issue Jun 29, 2014
When cloning a stream, the data is already guaranteed to be in a consistent
state, so there's no need to perform a zeroing. This prevents segfaults as seen
in #15231

Closes #15231
bors added a commit to rust-lang-ci/rust that referenced this issue Jul 17, 2023
…e-2, r=lowr

internal: Migrate more assists to use the structured snippet API

Continuing from rust-lang#14979

Migrates the following assists:
- `generate_derive`
- `wrap_return_type_in_result`
- `generate_delegate_methods`

As a bonus, `generate_delegate_methods` now generates the function and impl block at the correct indentation 🎉.
bors added a commit to rust-lang-ci/rust that referenced this issue Jul 17, 2023
…e-3, r=lowr

internal: Migrate assists to the structured snippet API, part 3

Continuing from rust-lang#15231

Migrates the following assists:
- `add_missing_match_arms`
- `fix_visibility`
- `promote_local_to_const`

The `add_missing_match_arms` changes are best reviewed commit-by-commit since they're relatively big changes compared to the rest of the commits.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants