From e0599ad53296819cc61e8db356ab18e2746de903 Mon Sep 17 00:00:00 2001 From: deuce <> Date: Thu, 30 Apr 2015 00:14:39 +0000 Subject: [PATCH] Properly lock vstat for all accesses (except some in SDL code where there is a potential problem with grabbing vstatlock). Call XCloseDisplay and terminate the X11 event thread on exit(). This still does not prevent the crash on exit for Linux. This appears to be an issue with SDL. --- src/conio/bitmap_con.c | 13 ++++++++- src/conio/sdl_con.c | 60 ++++++++++++++++++++++++++++++++-------- src/conio/x_cio.c | 4 +++ src/conio/x_events.c | 63 ++++++++++++++++++++++++++++++++++++++---- src/conio/x_events.h | 3 ++ 5 files changed, 125 insertions(+), 18 deletions(-) diff --git a/src/conio/bitmap_con.c b/src/conio/bitmap_con.c index 16c3cd593d..fac5053337 100644 --- a/src/conio/bitmap_con.c +++ b/src/conio/bitmap_con.c @@ -267,6 +267,7 @@ int bitmap_getvideoflags(void) { int flags=0; + pthread_mutex_lock(&vstatlock); if(vstat.bright_background) flags |= CIOLIB_VIDEO_BGBRIGHT; if(vstat.no_bright) @@ -277,11 +278,13 @@ int bitmap_getvideoflags(void) flags |= CIOLIB_VIDEO_NOBLINK; if(vstat.blink_altcharset) flags |= CIOLIB_VIDEO_BLINKALTCHARS; + pthread_mutex_unlock(&vstatlock); return(flags); } void bitmap_setvideoflags(int flags) { + pthread_mutex_lock(&vstatlock); if(flags & CIOLIB_VIDEO_BGBRIGHT) vstat.bright_background=1; else @@ -306,6 +309,7 @@ void bitmap_setvideoflags(int flags) vstat.blink_altcharset=1; else vstat.blink_altcharset=0; + pthread_mutex_unlock(&vstatlock); } int bitmap_movetext(int x, int y, int ex, int ey, int tox, int toy) @@ -601,13 +605,20 @@ int bitmap_getfont(void) void bitmap_setscaling(int new_value) { + pthread_mutex_lock(&vstatlock); if(new_value > 0) vstat.scaling = new_value; + pthread_mutex_unlock(&vstatlock); } int bitmap_getscaling(void) { - return vstat.scaling; + int ret; + + pthread_mutex_lock(&vstatlock); + ret = vstat.scaling; + pthread_mutex_unlock(&vstatlock); + return ret; } /* Called from event thread only */ diff --git a/src/conio/sdl_con.c b/src/conio/sdl_con.c index 80cbf48d7f..9e880eeeca 100644 --- a/src/conio/sdl_con.c +++ b/src/conio/sdl_con.c @@ -626,13 +626,18 @@ void sdl_flush(void) int sdl_init_mode(int mode) { - int oldcols=vstat.cols; + int oldcols; + + pthread_mutex_lock(&vstatlock); + oldcols = vstat.cols; + pthread_mutex_unlock(&vstatlock); sdl_user_func_ret(SDL_USEREVENT_FLUSH); bitmap_init_mode(mode, &bitmap_width, &bitmap_height); /* Deal with 40 col doubling */ + pthread_mutex_lock(&vstatlock); if(yuv.enabled) { vstat.scaling=2; } @@ -649,6 +654,7 @@ int sdl_init_mode(int mode) vstat.scaling = 1; if(vstat.vmultiplier < 1) vstat.vmultiplier = 1; + pthread_mutex_unlock(&vstatlock); sdl_user_func_ret(SDL_USEREVENT_SETVIDMODE); @@ -881,8 +887,8 @@ int sdl_setup_yuv_colours(void) void setup_surfaces(void) { - int char_width=vstat.charwidth*vstat.cols*vstat.scaling; - int char_height=vstat.charheight*vstat.rows*vstat.scaling*vstat.vmultiplier; + int char_width; + int char_height; int flags=SDL_HWSURFACE|SDL_ANYFORMAT; SDL_Surface *tmp_rect; SDL_Event ev; @@ -893,6 +899,10 @@ void setup_surfaces(void) flags |= SDL_RESIZABLE; sdl.mutexP(win_mutex); + pthread_mutex_lock(&vstatlock); + char_width=vstat.charwidth*vstat.cols*vstat.scaling; + char_height=vstat.charheight*vstat.rows*vstat.scaling*vstat.vmultiplier; + if(yuv.enabled) { if(!yuv.win_width) yuv.win_width=vstat.charwidth*vstat.cols; @@ -905,6 +915,7 @@ void setup_surfaces(void) } else win=sdl.SetVideoMode(char_width,char_height,8,flags); + pthread_mutex_unlock(&vstatlock); #if !defined(NO_X) && defined(__unix__) if(sdl_x11available && sdl_using_x11) { @@ -1466,41 +1477,58 @@ int sdl_mouse_thread(void *data) int win_to_text_xpos(int winpos) { + int ret; + if(yuv.enabled) { - int ret; sdl.mutexP(win_mutex); + pthread_mutex_lock(&vstatlock); ret = winpos*vstat.cols/win->w+1; + pthread_mutex_unlock(&vstatlock); sdl.mutexV(win_mutex); return(ret); } - else - return(winpos/(vstat.charwidth*vstat.scaling)+1); + else { + pthread_mutex_lock(&vstatlock); + ret = winpos/(vstat.charwidth*vstat.scaling)+1; + pthread_mutex_unlock(&vstatlock); + return ret; + } } int win_to_text_ypos(int winpos) { - if(yuv.enabled) { - int ret; + int ret; + if(yuv.enabled) { sdl.mutexP(win_mutex); + pthread_mutex_lock(&vstatlock); ret = winpos*vstat.rows/win->h+1; + pthread_mutex_unlock(&vstatlock); sdl.mutexV(win_mutex); return(ret); } - else - return(winpos/(vstat.charheight*vstat.scaling*vstat.vmultiplier)+1); + else { + pthread_mutex_lock(&vstatlock); + ret = winpos/(vstat.charheight*vstat.scaling*vstat.vmultiplier)+1; + pthread_mutex_unlock(&vstatlock); + return ret; + } } int sdl_video_event_thread(void *data) { SDL_Event ev; int new_scaling = -1; - int old_scaling = vstat.scaling; + int old_scaling; SDL_Rect *upd_rects=NULL; int rectspace=0; int rectsused=0; + pthread_mutex_lock(&vstatlock); + old_scaling = vstat.scaling; + pthread_mutex_unlock(&vstatlock); + if(!init_sdl_video()) { char driver[16]; if(sdl.VideoDriverName(driver, sizeof(driver))!=NULL) { @@ -1522,6 +1550,7 @@ int sdl_video_event_thread(void *data) while(1) { if(sdl.PollEvent(&ev)!=1) { + // TODO: Still accessing vstat without holding the lock... if (new_scaling != -1 || vstat.scaling != old_scaling) { if (new_scaling == -1) new_scaling = vstat.scaling; @@ -1594,8 +1623,11 @@ int sdl_video_event_thread(void *data) yuv.win_height=ev.resize.h; new_scaling = 2; } - else + else { + pthread_mutex_lock(&vstatlock); new_scaling = (int)(ev.resize.w/(vstat.charwidth*vstat.cols)); + pthread_mutex_unlock(&vstatlock); + } } break; case SDL_VIDEOEXPOSE: @@ -1644,6 +1676,7 @@ int sdl_video_event_thread(void *data) break; } sdl.mutexP(newrect_mutex); + pthread_mutex_lock(&vstatlock); for(y=0; y<rect->height; y++) { offset=y*rect->width; for(x=0; x<rect->width; x++) { @@ -1676,6 +1709,7 @@ int sdl_video_event_thread(void *data) rectsused=0; } } + pthread_mutex_unlock(&vstatlock); sdl.mutexV(newrect_mutex); sdl.mutexV(win_mutex); free(rect->data); @@ -1735,6 +1769,7 @@ int sdl_video_event_thread(void *data) free(ev.user.data1); break; case SDL_USEREVENT_SETVIDMODE: + pthread_mutex_lock(&vstatlock); if(!yuv.enabled) { rectspace=vstat.cols*vstat.rows+vstat.cols; rectsused=0; @@ -1749,6 +1784,7 @@ int sdl_video_event_thread(void *data) } new_scaling = -1; old_scaling = vstat.scaling; + pthread_mutex_unlock(&vstatlock); setup_surfaces(); sdl_ufunc_retval=0; sdl.SemPost(sdl_ufunc_ret); diff --git a/src/conio/x_cio.c b/src/conio/x_cio.c index 1a13dabef0..12baa80636 100644 --- a/src/conio/x_cio.c +++ b/src/conio/x_cio.c @@ -268,6 +268,10 @@ int x_init(void) xp_dlclose(dl); return(-1); } + if((x11.XCloseDisplay=xp_dlsym(dl,XCloseDisplay))==NULL) { + xp_dlclose(dl); + return(-1); + } if((x11.XCreateSimpleWindow=xp_dlsym(dl,XCreateSimpleWindow))==NULL) { xp_dlclose(dl); return(-1); diff --git a/src/conio/x_events.c b/src/conio/x_events.c index 164319fc70..77998c7e37 100644 --- a/src/conio/x_events.c +++ b/src/conio/x_events.c @@ -48,6 +48,8 @@ int x11_window_ypos; int x11_window_width; int x11_window_height; int x11_initialized=0; +sem_t event_thread_complete; +int terminate = 0; /* * Local variables */ @@ -210,8 +212,10 @@ static int init_window() white=WhitePixel(dpy, DefaultScreen(dpy)); /* Create window, but defer setting a size and GC. */ + pthread_mutex_lock(&vstatlock); win = x11.XCreateSimpleWindow(dpy, DefaultRootWindow(dpy), 0, 0, 640*vstat.scaling, 400*vstat.scaling*vstat.vmultiplier, 2, black, black); + pthread_mutex_unlock(&vstatlock); wmhints=x11.XAllocWMHints(); if(wmhints) { @@ -265,8 +269,10 @@ static void map_window() exit(1); } + pthread_mutex_lock(&vstatlock); sh->base_width = bitmap_width*vstat.scaling; sh->base_height = bitmap_height*vstat.scaling*vstat.vmultiplier; + pthread_mutex_unlock(&vstatlock); sh->min_width = sh->width_inc = sh->min_aspect.x = sh->max_aspect.x = bitmap_width; sh->min_height = sh->height_inc = sh->min_aspect.y = sh->max_aspect.y = bitmap_height; @@ -285,18 +291,26 @@ static void map_window() /* Resize the window. This function is called after a mode change. */ static void resize_window() { + pthread_mutex_lock(&vstatlock); x11.XResizeWindow(dpy, win, bitmap_width*vstat.scaling, bitmap_height*vstat.scaling*vstat.vmultiplier); + pthread_mutex_unlock(&vstatlock); + return; } static int init_mode(int mode) { - int oldcols=vstat.cols; + int oldcols; int oldwidth=bitmap_width; int oldheight=bitmap_height; + pthread_mutex_lock(&vstatlock); + oldcols=vstat.cols; + pthread_mutex_unlock(&vstatlock); + bitmap_init_mode(mode, &bitmap_width, &bitmap_height); + pthread_mutex_lock(&vstatlock); /* Deal with 40 col doubling */ if(oldcols != vstat.cols) { if(oldcols == 40) @@ -307,6 +321,7 @@ static int init_mode(int mode) if(vstat.scaling < 1) vstat.scaling = 1; + pthread_mutex_unlock(&vstatlock); map_window(); /* Resize window if necessary. */ @@ -323,10 +338,12 @@ static int video_init() /* If we are running under X, get a connection to the X server and create an empty window of size (1, 1). It makes a couple of init functions a lot easier. */ + pthread_mutex_lock(&vstatlock); if(vstat.scaling<1) vstat.scaling=1; if(vstat.vmultiplier<1) vstat.vmultiplier=1; + pthread_mutex_unlock(&vstatlock); if(init_window()) return(-1); @@ -349,6 +366,8 @@ static void local_draw_rect(struct update_rect *rect) #if 0 /* Draw solid colour rectangles... */ int rectw, recth, rectc, y2; + + pthread_mutex_lock(&vstatlock); for(y=0; y<rect->height; y++) { for(x=0; x<rect->width; x++) { rectc=rect->data[y*rect->width+x]; @@ -376,8 +395,10 @@ static void local_draw_rect(struct update_rect *rect) x11.XFillRectangle(dpy, win, gca[rectc], (rect->x+x)*vstat.scaling, (rect->y+y)*vstat.scaling*vstat.vmultiplier, rectw*vstat.scaling, recth*vstat.scaling*vstat.vmultiplier); } } + pthread_mutex_unlock(&vstatlock); #else #if 1 /* XImage */ + pthread_mutex_lock(&vstatlock); xim=x11.XCreateImage(dpy,&visual,depth,ZPixmap,0,NULL,rect->width*vstat.scaling,rect->height*vstat.scaling*vstat.vmultiplier,32,0); xim->data=(char *)malloc(xim->bytes_per_line*rect->height*vstat.scaling*vstat.vmultiplier); for(y=0;y<rect->height;y++) { @@ -395,6 +416,7 @@ static void local_draw_rect(struct update_rect *rect) } x11.XPutImage(dpy,win,gca[0],xim,0,0,rect->x*vstat.scaling,rect->y*vstat.scaling*vstat.vmultiplier,rect->width*vstat.scaling,rect->height*vstat.scaling*vstat.vmultiplier); + pthread_mutex_unlock(&vstatlock); #ifdef XDestroyImage XDestroyImage(xim); #else @@ -402,11 +424,13 @@ static void local_draw_rect(struct update_rect *rect) #endif #else /* XFillRectangle */ + pthread_mutex_lock(&vstatlock); for(y=0;y<rect->height;y++) { for(x=0; x<rect->width; x++) { x11.XFillRectangle(dpy, win, gca[rect->data[y*rect->width+x]], (rect->x+x)*vstat.scaling, (rect->y+y)*vstat.scaling*vstat.vmultiplier, vstat.scaling, vstat.scaling*vstat.vmultiplier); } } + pthread_mutex_unlock(&vstatlock); #endif #endif free(rect->data); @@ -418,9 +442,12 @@ static void handle_resize_event(int width, int height) int newFSW=1; // No change + pthread_mutex_lock(&vstatlock); if((width == vstat.charwidth * vstat.cols * vstat.scaling) - && (height == vstat.charheight * vstat.rows * vstat.scaling*vstat.vmultiplier)) + && (height == vstat.charheight * vstat.rows * vstat.scaling*vstat.vmultiplier)) { + pthread_mutex_unlock(&vstatlock); return; + } newFSH=width/bitmap_width; newFSW=height/bitmap_height; @@ -440,15 +467,20 @@ static void handle_resize_event(int width, int height) * Otherwise, we can simply resend everything */ if((width % (vstat.charwidth * vstat.cols) != 0) - || (height % (vstat.charheight * vstat.rows) != 0)) + || (height % (vstat.charheight * vstat.rows) != 0)) { + pthread_mutex_unlock(&vstatlock); resize_window(); + } + else + pthread_mutex_unlock(&vstatlock); send_rectangle(0,0,bitmap_width,bitmap_height,TRUE); } -static void expose_rect(x,y,width,height) +static void expose_rect(int x, int y, int width, int height) { int sx,sy,ex,ey; + pthread_mutex_lock(&vstatlock); sx=x/vstat.scaling; sy=y/(vstat.scaling*vstat.vmultiplier); @@ -462,6 +494,7 @@ static void expose_rect(x,y,width,height) } ex=ex/vstat.scaling; ey=ey/(vstat.scaling*vstat.vmultiplier); + pthread_mutex_unlock(&vstatlock); send_rectangle(sx, sy, ex-sx+1, ey-sy+1, TRUE); } @@ -565,6 +598,7 @@ static int x11_event(XEvent *ev) { XMotionEvent *me = (XMotionEvent *)ev; + pthread_mutex_lock(&vstatlock); me->x/=vstat.scaling; me->x/=vstat.charwidth; me->y/=vstat.scaling; @@ -580,6 +614,7 @@ static int x11_event(XEvent *ev) me->x=vstat.cols; if(me->y>vstat.rows+1) me->y=vstat.rows+1; + pthread_mutex_unlock(&vstatlock); ciomouse_gotevent(CIOLIB_MOUSE_MOVE,me->x,me->y); } break; @@ -587,6 +622,7 @@ static int x11_event(XEvent *ev) { XButtonEvent *be = (XButtonEvent *)ev; + pthread_mutex_lock(&vstatlock); be->x/=vstat.scaling; be->x/=vstat.charwidth; be->y/=vstat.scaling; @@ -602,6 +638,7 @@ static int x11_event(XEvent *ev) be->x=vstat.cols; if(be->y>vstat.rows+1) be->y=vstat.rows+1; + pthread_mutex_unlock(&vstatlock); if (be->button <= 3) { ciomouse_gotevent(CIOLIB_BUTTON_RELEASE(be->button),be->x,be->y); } @@ -611,6 +648,7 @@ static int x11_event(XEvent *ev) { XButtonEvent *be = (XButtonEvent *)ev; + pthread_mutex_lock(&vstatlock); be->x/=vstat.scaling; be->x/=vstat.charwidth; be->y/=vstat.scaling; @@ -626,6 +664,7 @@ static int x11_event(XEvent *ev) be->x=vstat.cols; if(be->y>vstat.rows+1) be->y=vstat.rows+1; + pthread_mutex_unlock(&vstatlock); if (be->button <= 3) { ciomouse_gotevent(CIOLIB_BUTTON_PRESS(be->button),be->x,be->y); } @@ -824,10 +863,20 @@ static int x11_event(XEvent *ev) void check_scaling(void) { + pthread_mutex_lock(&vstatlock); if (old_scaling != vstat.scaling) { + pthread_mutex_unlock(&vstatlock); resize_window(); + pthread_mutex_lock(&vstatlock); old_scaling = vstat.scaling; } + pthread_mutex_unlock(&vstatlock); +} + +static void x11_terminate_event_thread(void) +{ + terminate = 1; + sem_wait(&event_thread_complete); } void x11_event_thread(void *args) @@ -844,6 +893,8 @@ void x11_event_thread(void *args) return; } x11_initialized=1; + sem_init(&event_thread_complete, 0, 0); + atexit(x11_terminate_event_thread); sem_post(&init_complete); if(local_pipe[0] > xfd) @@ -851,7 +902,7 @@ void x11_event_thread(void *args) else high_fd=xfd; - for (;;) { + for (;!terminate;) { check_scaling(); tv.tv_sec=0; @@ -956,4 +1007,6 @@ void x11_event_thread(void *args) } } } + x11.XCloseDisplay(dpy); + sem_post(&event_thread_complete); } diff --git a/src/conio/x_events.h b/src/conio/x_events.h index 366aa51a3f..6e55daf619 100644 --- a/src/conio/x_events.h +++ b/src/conio/x_events.h @@ -57,6 +57,7 @@ struct x11 { Pixmap (*XCreateBitmapFromData) (Display*, Drawable, _Xconst char*, unsigned int, unsigned int); Status (*XAllocColor) (Display*, Colormap, XColor*); Display*(*XOpenDisplay) (_Xconst char*); + int (*XCloseDisplay) (Display*); Window (*XCreateSimpleWindow) (Display*, Window, int, int, unsigned int, unsigned int, unsigned int, unsigned long, unsigned long); GC (*XCreateGC) (Display*, Drawable, unsigned long, XGCValues*); int (*XSelectInput) (Display*, Window, long); @@ -98,6 +99,8 @@ extern sem_t pastebuf_set; extern sem_t pastebuf_used; extern sem_t init_complete; extern sem_t mode_set; +extern sem_t event_thread_complete; +extern int terminate; extern int x11_window_xpos; extern int x11_window_ypos; extern int x11_window_width; -- GitLab