Add wrappers for safer error recovery during initialization

src/win32/init.rs

@@ -3,9 +3,12 @@ use std::ptr;
 use std::mem;
 use std::os;
 use std::thread;
+
 use super::callback;
 use super::Window;
 use super::MonitorID;
+use super::ContextWrapper;
+use super::WindowWrapper;
 
 use Api;
 use BuilderAttribs;
@@ -116,37 +119,32 @@ unsafe fn init(title: Vec<u16>, builder: BuilderAttribs<'static>,
 
             if handle.is_null() {
                 return Err(OsError(format!("CreateWindowEx function failed: {}",
-                    os::error_string(os::errno()))));
+                                           os::error_string(os::errno()))));
             }
 
-            handle
+            let hdc = user32::GetDC(handle);
-        };
-
-        // getting the HDC of the dummy window
-        let dummy_hdc = {
-            let hdc = user32::GetDC(dummy_window);
             if hdc.is_null() {
                 let err = Err(OsError(format!("GetDC function failed: {}",
-                    os::error_string(os::errno()))));
+                                              os::error_string(os::errno()))));
-                user32::DestroyWindow(dummy_window);
                 return err;
             }
-            hdc
+
+            WindowWrapper(handle, hdc)
         };
 
         // getting the pixel format that we will use and setting it
         {
-            let formats = enumerate_native_pixel_formats(dummy_hdc);
+            let formats = enumerate_native_pixel_formats(&dummy_window);
             let (id, _) = builder.choose_pixel_format(formats.into_iter().map(|(a, b)| (b, a)));
-            try!(set_pixel_format(dummy_hdc, id));
+            try!(set_pixel_format(&dummy_window, id));
         }
 
         // creating the dummy OpenGL context
-        let dummy_context = try!(create_context(None, dummy_hdc, None));
+        let dummy_context = try!(create_context(None, &dummy_window, None));
 
         // making context current
-        gl::wgl::MakeCurrent(dummy_hdc as *const libc::c_void,
+        gl::wgl::MakeCurrent(dummy_window.1 as *const libc::c_void,
-                             dummy_context as *const libc::c_void);
+                             dummy_context.0 as *const libc::c_void);
 
         // loading the extra WGL functions
         let extra_functions = gl::wgl_extra::Wgl::load_with(|addr| {
@@ -161,10 +159,6 @@ unsafe fn init(title: Vec<u16>, builder: BuilderAttribs<'static>,
         // removing current context
         gl::wgl::MakeCurrent(ptr::null(), ptr::null());
 
-        // destroying the context and the window
-        gl::wgl::DeleteContext(dummy_context as *const libc::c_void);
-        user32::DestroyWindow(dummy_window);
-
         // returning the address
         extra_functions
     };
@@ -197,39 +191,34 @@ unsafe fn init(title: Vec<u16>, builder: BuilderAttribs<'static>,
                                        os::error_string(os::errno()))));
         }
 
-        handle
+        let hdc = user32::GetDC(handle);
-    };
-
-    // getting the HDC of the window
-    let hdc = {
-        let hdc = user32::GetDC(real_window);
         if hdc.is_null() {
-            let err = Err(OsError(format!("GetDC function failed: {}",
+            return Err(OsError(format!("GetDC function failed: {}",
-                                          os::error_string(os::errno()))));
+                                       os::error_string(os::errno()))));
-            user32::DestroyWindow(real_window);
-            return err;
         }
-        hdc
+
+        WindowWrapper(handle, hdc)
     };
 
     // calling SetPixelFormat
     {
         let formats = if extra_functions.GetPixelFormatAttribivARB.is_loaded() {
-            enumerate_arb_pixel_formats(&extra_functions, hdc)
+            enumerate_arb_pixel_formats(&extra_functions, &real_window)
         } else {
-            enumerate_native_pixel_formats(hdc)
+            enumerate_native_pixel_formats(&real_window)
         };
 
         let (id, _) = builder.choose_pixel_format(formats.into_iter().map(|(a, b)| (b, a)));
-        try!(set_pixel_format(hdc, id));
+        try!(set_pixel_format(&real_window, id));
     }
 
     // creating the OpenGL context
-    let context = try!(create_context(Some((&extra_functions, &builder)), hdc, builder_sharelists));
+    let context = try!(create_context(Some((&extra_functions, &builder)), &real_window,
+                                      builder_sharelists));
 
     // calling SetForegroundWindow if fullscreen
     if builder.monitor.is_some() {
-        user32::SetForegroundWindow(real_window);
+        user32::SetForegroundWindow(real_window.0);
     }
 
     // filling the WINDOW task-local storage
@@ -237,7 +226,7 @@ unsafe fn init(title: Vec<u16>, builder: BuilderAttribs<'static>,
         let (tx, rx) = channel();
         let mut tx = Some(tx);
         callback::WINDOW.with(|window| {
-            (*window.borrow_mut()) = Some((real_window, tx.take().unwrap()));
+            (*window.borrow_mut()) = Some((real_window.0, tx.take().unwrap()));
         });
         rx
     };
@@ -248,10 +237,9 @@ unsafe fn init(title: Vec<u16>, builder: BuilderAttribs<'static>,
     // handling vsync
     if builder.vsync {
         if extra_functions.SwapIntervalEXT.is_loaded() {
-            gl::wgl::MakeCurrent(hdc as *const libc::c_void, context as *const libc::c_void);
+            gl::wgl::MakeCurrent(real_window.1 as *const libc::c_void,
+                                 context.0 as *const libc::c_void);
             if extra_functions.SwapIntervalEXT(1) == 0 {
-                gl::wgl::DeleteContext(context as *const libc::c_void);
-                user32::DestroyWindow(real_window);
                 return Err(OsError(format!("wglSwapIntervalEXT failed")));
             }
 
@@ -264,7 +252,6 @@ unsafe fn init(title: Vec<u16>, builder: BuilderAttribs<'static>,
     // building the struct
     Ok(Window {
         window: real_window,
-        hdc: hdc as winapi::HDC,
         context: context,
         gl_library: gl_library,
         events_receiver: events_receiver,
@@ -332,8 +319,8 @@ unsafe fn switch_to_fullscreen(rect: &mut winapi::RECT, monitor: &MonitorID)
 }
 
 unsafe fn create_context(extra: Option<(&gl::wgl_extra::Wgl, &BuilderAttribs<'static>)>,
-                         hdc: winapi::HDC, share: Option<winapi::HGLRC>)
+                         hdc: &WindowWrapper, share: Option<winapi::HGLRC>)
-                         -> Result<winapi::HGLRC, CreationError>
+                         -> Result<ContextWrapper, CreationError>
 {
     let share = share.unwrap_or(ptr::null_mut());
 
@@ -365,7 +352,7 @@ unsafe fn create_context(extra: Option<(&gl::wgl_extra::Wgl, &BuilderAttribs<'st
 
             attributes.push(0);
 
-            Some(extra_functions.CreateContextAttribsARB(hdc as *const libc::c_void,
+            Some(extra_functions.CreateContextAttribsARB(hdc.1 as *const libc::c_void,
                                                          share as *const libc::c_void,
                                                          attributes.as_slice().as_ptr()))
 
@@ -379,7 +366,7 @@ unsafe fn create_context(extra: Option<(&gl::wgl_extra::Wgl, &BuilderAttribs<'st
     let ctxt = match ctxt {
         Some(ctxt) => ctxt,
         None => {
-            let ctxt = gl::wgl::CreateContext(hdc as *const libc::c_void);
+            let ctxt = gl::wgl::CreateContext(hdc.1 as *const libc::c_void);
             if !ctxt.is_null() && !share.is_null() {
                 gl::wgl::ShareLists(share as *const libc::c_void, ctxt);
             };
@@ -392,19 +379,19 @@ unsafe fn create_context(extra: Option<(&gl::wgl_extra::Wgl, &BuilderAttribs<'st
                            os::error_string(os::errno()))));
     }
 
-    Ok(ctxt as winapi::HGLRC)
+    Ok(ContextWrapper(ctxt as winapi::HGLRC))
 }
 
-unsafe fn enumerate_native_pixel_formats(hdc: winapi::HDC) -> Vec<(PixelFormat, libc::c_int)> {
+unsafe fn enumerate_native_pixel_formats(hdc: &WindowWrapper) -> Vec<(PixelFormat, libc::c_int)> {
     let size_of_pxfmtdescr = mem::size_of::<winapi::PIXELFORMATDESCRIPTOR>() as u32;
-    let num = gdi32::DescribePixelFormat(hdc, 1, size_of_pxfmtdescr, ptr::null_mut());
+    let num = gdi32::DescribePixelFormat(hdc.1, 1, size_of_pxfmtdescr, ptr::null_mut());
 
     let mut result = Vec::new();
 
     for index in (0 .. num) {
         let mut output: winapi::PIXELFORMATDESCRIPTOR = mem::zeroed();
         
-        if gdi32::DescribePixelFormat(hdc, index, size_of_pxfmtdescr, &mut output) == 0 {
+        if gdi32::DescribePixelFormat(hdc.1, index, size_of_pxfmtdescr, &mut output) == 0 {
             continue;
         }
 
@@ -438,12 +425,12 @@ unsafe fn enumerate_native_pixel_formats(hdc: winapi::HDC) -> Vec<(PixelFormat,
     result
 }
 
-unsafe fn enumerate_arb_pixel_formats(extra: &gl::wgl_extra::Wgl, hdc: winapi::HDC)
+unsafe fn enumerate_arb_pixel_formats(extra: &gl::wgl_extra::Wgl, hdc: &WindowWrapper)
                                       -> Vec<(PixelFormat, libc::c_int)>
 {
     let get_info = |index: u32, attrib: u32| {
         let mut value = mem::uninitialized();
-        extra.GetPixelFormatAttribivARB(hdc as *const libc::c_void, index as libc::c_int,
+        extra.GetPixelFormatAttribivARB(hdc.1 as *const libc::c_void, index as libc::c_int,
                                         0, 1, [attrib as libc::c_int].as_ptr(),
                                         &mut value);
         value as u32
@@ -489,17 +476,17 @@ unsafe fn enumerate_arb_pixel_formats(extra: &gl::wgl_extra::Wgl, hdc: winapi::H
     result
 }
 
-unsafe fn set_pixel_format(hdc: winapi::HDC, id: libc::c_int) -> Result<(), CreationError> {
+unsafe fn set_pixel_format(hdc: &WindowWrapper, id: libc::c_int) -> Result<(), CreationError> {
     let mut output: winapi::PIXELFORMATDESCRIPTOR = mem::zeroed();
 
-    if gdi32::DescribePixelFormat(hdc, id, mem::size_of::<winapi::PIXELFORMATDESCRIPTOR>()
+    if gdi32::DescribePixelFormat(hdc.1, id, mem::size_of::<winapi::PIXELFORMATDESCRIPTOR>()
                                   as winapi::UINT, &mut output) == 0
     {
         return Err(OsError(format!("DescribePixelFormat function failed: {}",
                                    os::error_string(os::errno()))));
     }
 
-    if gdi32::SetPixelFormat(hdc, id, &output) == 0 {
+    if gdi32::SetPixelFormat(hdc.1, id, &output) == 0 {
         return Err(OsError(format!("SetPixelFormat function failed: {}",
                                    os::error_string(os::errno()))));
     }

src/win32/mod.rs

@@ -26,13 +26,10 @@ mod monitor;
 /// The Win32 implementation of the main `Window` object.
 pub struct Window {
     /// Main handle for the window.
-    window: winapi::HWND,
+    window: WindowWrapper,
-
-    /// This represents a "draw context" for the surface of the window.
-    hdc: winapi::HDC,
 
     /// OpenGL context.
-    context: winapi::HGLRC,
+    context: ContextWrapper,
 
     /// Binded to `opengl32.dll`.
     ///
@@ -50,12 +47,25 @@ pub struct Window {
 unsafe impl Send for Window {}
 unsafe impl Sync for Window {}
 
-impl Window {
+/// A simple wrapper that destroys the context when it is destroyed.
-    /// See the docs in the crate root file.
+struct ContextWrapper(pub winapi::HGLRC);
-    pub fn new(builder: BuilderAttribs) -> Result<Window, CreationError> {
+
-        let (builder, sharing) = builder.extract_non_static();
+impl Drop for ContextWrapper {
-        let sharing = sharing.map(|w| init::ContextHack(w.context));
+    fn drop(&mut self) {
-        init::new_window(builder, sharing)
+        unsafe {
+            gl::wgl::DeleteContext(self.0 as *const libc::c_void);
+        }
+    }
+}
+
+/// A simple wrapper that destroys the window when it is destroyed.
+struct WindowWrapper(pub winapi::HWND, pub winapi::HDC);
+
+impl Drop for WindowWrapper {
+    fn drop(&mut self) {
+        unsafe {
+            user32::DestroyWindow(self.0);
+        }
     }
 }
 
@@ -69,6 +79,13 @@ impl WindowProxy {
 }
 
 impl Window {
+    /// See the docs in the crate root file.
+    pub fn new(builder: BuilderAttribs) -> Result<Window, CreationError> {
+        let (builder, sharing) = builder.extract_non_static();
+        let sharing = sharing.map(|w| init::ContextHack(w.context.0));
+        init::new_window(builder, sharing)
+    }
+
     /// See the docs in the crate root file.
     pub fn is_closed(&self) -> bool {
         use std::sync::atomic::Ordering::Relaxed;
@@ -80,7 +97,7 @@ impl Window {
     /// Calls SetWindowText on the HWND.
     pub fn set_title(&self, text: &str) {
         unsafe {
-            user32::SetWindowTextW(self.window,
+            user32::SetWindowTextW(self.window.0,
                 text.utf16_units().chain(Some(0).into_iter())
                 .collect::<Vec<u16>>().as_ptr() as winapi::LPCWSTR);
         }
@@ -88,13 +105,13 @@ impl Window {
 
     pub fn show(&self) {
         unsafe {
-            user32::ShowWindow(self.window, winapi::SW_SHOW);
+            user32::ShowWindow(self.window.0, winapi::SW_SHOW);
         }
     }
 
     pub fn hide(&self) {
         unsafe {
-            user32::ShowWindow(self.window, winapi::SW_HIDE);
+            user32::ShowWindow(self.window.0, winapi::SW_HIDE);
         }
     }
 
@@ -105,7 +122,7 @@ impl Window {
         let mut placement: winapi::WINDOWPLACEMENT = unsafe { mem::zeroed() };
         placement.length = mem::size_of::<winapi::WINDOWPLACEMENT>() as winapi::UINT;
 
-        if unsafe { user32::GetWindowPlacement(self.window, &mut placement) } == 0 {
+        if unsafe { user32::GetWindowPlacement(self.window.0, &mut placement) } == 0 {
             return None
         }
 
@@ -118,9 +135,9 @@ impl Window {
         use libc;
 
         unsafe {
-            user32::SetWindowPos(self.window, ptr::null_mut(), x as libc::c_int, y as libc::c_int,
+            user32::SetWindowPos(self.window.0, ptr::null_mut(), x as libc::c_int, y as libc::c_int,
                 0, 0, winapi::SWP_NOZORDER | winapi::SWP_NOSIZE);
-            user32::UpdateWindow(self.window);
+            user32::UpdateWindow(self.window.0);
         }
     }
 
@@ -129,7 +146,7 @@ impl Window {
         use std::mem;
         let mut rect: winapi::RECT = unsafe { mem::uninitialized() };
 
-        if unsafe { user32::GetClientRect(self.window, &mut rect) } == 0 {
+        if unsafe { user32::GetClientRect(self.window.0, &mut rect) } == 0 {
             return None
         }
 
@@ -144,7 +161,7 @@ impl Window {
         use std::mem;
         let mut rect: winapi::RECT = unsafe { mem::uninitialized() };
 
-        if unsafe { user32::GetWindowRect(self.window, &mut rect) } == 0 {
+        if unsafe { user32::GetWindowRect(self.window.0, &mut rect) } == 0 {
             return None
         }
 
@@ -159,9 +176,9 @@ impl Window {
         use libc;
 
         unsafe {
-            user32::SetWindowPos(self.window, ptr::null_mut(), 0, 0, x as libc::c_int,
+            user32::SetWindowPos(self.window.0, ptr::null_mut(), 0, 0, x as libc::c_int,
                 y as libc::c_int, winapi::SWP_NOZORDER | winapi::SWP_NOREPOSITION);
-            user32::UpdateWindow(self.window);
+            user32::UpdateWindow(self.window.0);
         }
     }
 
@@ -186,7 +203,8 @@ impl Window {
     /// See the docs in the crate root file.
     pub unsafe fn make_current(&self) {
         // TODO: check return value
-        gl::wgl::MakeCurrent(self.hdc as *const libc::c_void, self.context as *const libc::c_void);
+        gl::wgl::MakeCurrent(self.window.1 as *const libc::c_void,
+                             self.context.0 as *const libc::c_void);
     }
 
     /// See the docs in the crate root file.
@@ -204,7 +222,7 @@ impl Window {
     /// See the docs in the crate root file.
     pub fn swap_buffers(&self) {
         unsafe {
-            gdi32::SwapBuffers(self.hdc);
+            gdi32::SwapBuffers(self.window.1);
         }
     }
 
@@ -213,7 +231,7 @@ impl Window {
     }
 
     pub fn platform_window(&self) -> *mut libc::c_void {
-        self.window as *mut libc::c_void
+        self.window.0 as *mut libc::c_void
     }
 
     /// See the docs in the crate root file.
@@ -280,10 +298,10 @@ impl<'a> Iterator for WaitEventsIterator<'a> {
 #[unsafe_destructor]
 impl Drop for Window {
     fn drop(&mut self) {
-        // we don't call MakeCurrent(0, 0) because we are not sure that the context
+        unsafe {
-        // is still the current one
+            // we don't call MakeCurrent(0, 0) because we are not sure that the context
-        unsafe { user32::PostMessageW(self.window, winapi::WM_DESTROY, 0, 0); }
+            // is still the current one
-        unsafe { gl::wgl::DeleteContext(self.context as *const libc::c_void); }
+            user32::PostMessageW(self.window.0, winapi::WM_DESTROY, 0, 0);
-        unsafe { user32::DestroyWindow(self.window); }
+        }
     }
 }