From 89419fb1a0cef42b63528e6930f4e545cfef4c95 Mon Sep 17 00:00:00 2001 From: Christian Beier Date: Sun, 21 Oct 2018 23:38:40 +0200 Subject: [PATCH 1/5] tightvnc-filetransfer: tie the download thread to the control structure re #242 --- libvncserver/tightvnc-filetransfer/handlefiletransferrequest.c | 3 +-- libvncserver/tightvnc-filetransfer/rfbtightproto.h | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libvncserver/tightvnc-filetransfer/handlefiletransferrequest.c b/libvncserver/tightvnc-filetransfer/handlefiletransferrequest.c index 0473783..8e38f88 100644 --- a/libvncserver/tightvnc-filetransfer/handlefiletransferrequest.c +++ b/libvncserver/tightvnc-filetransfer/handlefiletransferrequest.c @@ -508,7 +508,6 @@ RunFileDownloadThread(void* client) void HandleFileDownload(rfbClientPtr cl, rfbTightClientPtr rtcp) { - pthread_t fileDownloadThread; FileTransferMsg fileDownloadMsg; memset(&fileDownloadMsg, 0, sizeof(FileTransferMsg)); @@ -521,7 +520,7 @@ HandleFileDownload(rfbClientPtr cl, rfbTightClientPtr rtcp) rtcp->rcft.rcfd.downloadInProgress = FALSE; rtcp->rcft.rcfd.downloadFD = -1; - if(pthread_create(&fileDownloadThread, NULL, RunFileDownloadThread, (void*) + if(pthread_create(&rtcp->rcft.rcfd.downloadThread, NULL, RunFileDownloadThread, (void*) cl) != 0) { FileTransferMsg ftm = GetFileDownLoadErrMsg(); diff --git a/libvncserver/tightvnc-filetransfer/rfbtightproto.h b/libvncserver/tightvnc-filetransfer/rfbtightproto.h index d0fe642..30fc5f5 100644 --- a/libvncserver/tightvnc-filetransfer/rfbtightproto.h +++ b/libvncserver/tightvnc-filetransfer/rfbtightproto.h @@ -148,6 +148,7 @@ typedef struct _rfbClientFileDownload { int downloadInProgress; unsigned long mTime; int downloadFD; + pthread_t downloadThread; } rfbClientFileDownload ; typedef struct _rfbClientFileUpload { From f8912fee5a58fb3975eda2589f6d4686f0c1ae68 Mon Sep 17 00:00:00 2001 From: Christian Beier Date: Sun, 21 Oct 2018 23:44:39 +0200 Subject: [PATCH 2/5] tightvnc-filetransfer: refactor CloseUndoneFileTransfer() into two functions ...for closing upload and download separately. re #242 --- libvncserver/tightvnc-filetransfer/filetransfermsg.c | 12 ++++++++++-- libvncserver/tightvnc-filetransfer/filetransfermsg.h | 3 ++- .../handlefiletransferrequest.c | 8 ++++---- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/libvncserver/tightvnc-filetransfer/filetransfermsg.c b/libvncserver/tightvnc-filetransfer/filetransfermsg.c index 5f84e7f..f674b92 100644 --- a/libvncserver/tightvnc-filetransfer/filetransfermsg.c +++ b/libvncserver/tightvnc-filetransfer/filetransfermsg.c @@ -672,7 +672,7 @@ ChkFileUploadWriteErr(rfbClientPtr cl, rfbTightClientPtr rtcp, char* pBuf) char reason[] = "Error writing file data"; int reasonLen = strlen(reason); ftm = CreateFileUploadErrMsg(reason, reasonLen); - CloseUndoneFileTransfer(cl, rtcp); + CloseUndoneFileUpload(cl, rtcp); } return ftm; } @@ -735,7 +735,7 @@ CreateFileUploadErrMsg(char* reason, unsigned int reasonLen) ******************************************************************************/ void -CloseUndoneFileTransfer(rfbClientPtr cl, rfbTightClientPtr rtcp) +CloseUndoneFileUpload(rfbClientPtr cl, rfbTightClientPtr rtcp) { /* TODO :: File Upload case is not handled currently */ /* TODO :: In case of concurrency we need to use Critical Section */ @@ -759,6 +759,14 @@ CloseUndoneFileTransfer(rfbClientPtr cl, rfbTightClientPtr rtcp) memset(rtcp->rcft.rcfu.fName, 0 , PATH_MAX); } +} + + +void +CloseUndoneFileDownload(rfbClientPtr cl, rfbTightClientPtr rtcp) +{ + if(cl == NULL) + return; if(rtcp->rcft.rcfd.downloadInProgress == TRUE) { rtcp->rcft.rcfd.downloadInProgress = FALSE; diff --git a/libvncserver/tightvnc-filetransfer/filetransfermsg.h b/libvncserver/tightvnc-filetransfer/filetransfermsg.h index 3b27bd0..bbb9148 100644 --- a/libvncserver/tightvnc-filetransfer/filetransfermsg.h +++ b/libvncserver/tightvnc-filetransfer/filetransfermsg.h @@ -51,7 +51,8 @@ FileTransferMsg ChkFileUploadWriteErr(rfbClientPtr cl, rfbTightClientPtr data, c void CreateDirectory(char* dirName); void FileUpdateComplete(rfbClientPtr cl, rfbTightClientPtr data); -void CloseUndoneFileTransfer(rfbClientPtr cl, rfbTightClientPtr data); +void CloseUndoneFileUpload(rfbClientPtr cl, rfbTightClientPtr data); +void CloseUndoneFileDownload(rfbClientPtr cl, rfbTightClientPtr data); void FreeFileTransferMsg(FileTransferMsg ftm); diff --git a/libvncserver/tightvnc-filetransfer/handlefiletransferrequest.c b/libvncserver/tightvnc-filetransfer/handlefiletransferrequest.c index 8e38f88..31163d0 100644 --- a/libvncserver/tightvnc-filetransfer/handlefiletransferrequest.c +++ b/libvncserver/tightvnc-filetransfer/handlefiletransferrequest.c @@ -492,7 +492,7 @@ RunFileDownloadThread(void* client) if(cl != NULL) { rfbCloseClient(cl); - CloseUndoneFileTransfer(cl, rtcp); + CloseUndoneFileDownload(cl, rtcp); } FreeFileTransferMsg(fileDownloadMsg); @@ -592,7 +592,7 @@ HandleFileDownloadCancelRequest(rfbClientPtr cl, rfbTightClientPtr rtcp) " reason <%s>\n", __FILE__, __FUNCTION__, reason); pthread_mutex_lock(&fileDownloadMutex); - CloseUndoneFileTransfer(cl, rtcp); + CloseUndoneFileDownload(cl, rtcp); pthread_mutex_unlock(&fileDownloadMutex); if(reason != NULL) { @@ -835,7 +835,7 @@ HandleFileUploadDataRequest(rfbClientPtr cl, rfbTightClientPtr rtcp) FreeFileTransferMsg(ftm); } - CloseUndoneFileTransfer(cl, rtcp); + CloseUndoneFileUpload(cl, rtcp); if(pBuf != NULL) { free(pBuf); @@ -935,7 +935,7 @@ HandleFileUploadFailedRequest(rfbClientPtr cl, rfbTightClientPtr rtcp) rfbLog("File [%s]: Method [%s]: File Upload Failed Request received:" " reason <%s>\n", __FILE__, __FUNCTION__, reason); - CloseUndoneFileTransfer(cl, rtcp); + CloseUndoneFileUpload(cl, rtcp); if(reason != NULL) { free(reason); From 73cb96fec028a576a5a24417b57723b55854ad7b Mon Sep 17 00:00:00 2001 From: Christian Beier Date: Sun, 21 Oct 2018 23:59:39 +0200 Subject: [PATCH 3/5] tightvnc-filetransfer: wait for download thread end in CloseUndoneFileDownload() ...and use it when deregistering the file transfer extension. Closes #242 --- libvncserver/tightvnc-filetransfer/filetransfermsg.c | 2 ++ libvncserver/tightvnc-filetransfer/rfbtightserver.c | 7 +++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/libvncserver/tightvnc-filetransfer/filetransfermsg.c b/libvncserver/tightvnc-filetransfer/filetransfermsg.c index f674b92..0003b11 100644 --- a/libvncserver/tightvnc-filetransfer/filetransfermsg.c +++ b/libvncserver/tightvnc-filetransfer/filetransfermsg.c @@ -770,6 +770,8 @@ CloseUndoneFileDownload(rfbClientPtr cl, rfbTightClientPtr rtcp) if(rtcp->rcft.rcfd.downloadInProgress == TRUE) { rtcp->rcft.rcfd.downloadInProgress = FALSE; + /* the thread will return if downloadInProgress is FALSE */ + pthread_join(rtcp->rcft.rcfd.downloadThread, NULL); if(rtcp->rcft.rcfd.downloadFD != -1) { close(rtcp->rcft.rcfd.downloadFD); diff --git a/libvncserver/tightvnc-filetransfer/rfbtightserver.c b/libvncserver/tightvnc-filetransfer/rfbtightserver.c index 67d4cb5..651d8fb 100644 --- a/libvncserver/tightvnc-filetransfer/rfbtightserver.c +++ b/libvncserver/tightvnc-filetransfer/rfbtightserver.c @@ -26,6 +26,7 @@ #include #include "rfbtightproto.h" #include "handlefiletransferrequest.h" +#include "filetransfermsg.h" /* * Get my data! @@ -448,9 +449,11 @@ rfbTightExtensionMsgHandler(struct _rfbClientRec* cl, void* data, void rfbTightExtensionClientClose(rfbClientPtr cl, void* data) { - if(data != NULL) + if(data != NULL) { + CloseUndoneFileUpload(cl, data); + CloseUndoneFileDownload(cl, data); free(data); - + } } void From 2d939267a176bf4976dbad36399638956ad8cc34 Mon Sep 17 00:00:00 2001 From: Christian Beier Date: Mon, 22 Oct 2018 00:39:50 +0200 Subject: [PATCH 4/5] tightvnc-filetransfer: when creating a new download thread, make sure the previous one ends re #242 --- libvncserver/tightvnc-filetransfer/handlefiletransferrequest.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libvncserver/tightvnc-filetransfer/handlefiletransferrequest.c b/libvncserver/tightvnc-filetransfer/handlefiletransferrequest.c index 31163d0..70e105f 100644 --- a/libvncserver/tightvnc-filetransfer/handlefiletransferrequest.c +++ b/libvncserver/tightvnc-filetransfer/handlefiletransferrequest.c @@ -517,8 +517,7 @@ HandleFileDownload(rfbClientPtr cl, rfbTightClientPtr rtcp) FreeFileTransferMsg(fileDownloadMsg); return; } - rtcp->rcft.rcfd.downloadInProgress = FALSE; - rtcp->rcft.rcfd.downloadFD = -1; + CloseUndoneFileDownload(cl, rtcp); if(pthread_create(&rtcp->rcft.rcfd.downloadThread, NULL, RunFileDownloadThread, (void*) cl) != 0) { From 495ffa3f3a213ab058eee1d7da48fa5ef71914d8 Mon Sep 17 00:00:00 2001 From: Christian Beier Date: Sat, 10 Nov 2018 17:33:00 +0100 Subject: [PATCH 5/5] tightvnc-filetransfer: do not close stuff from within a thread ... as this crashes badly and the client is closed by the main thread machinery afterwards. re #242 --- .../tightvnc-filetransfer/handlefiletransferrequest.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/libvncserver/tightvnc-filetransfer/handlefiletransferrequest.c b/libvncserver/tightvnc-filetransfer/handlefiletransferrequest.c index 70e105f..71fb085 100644 --- a/libvncserver/tightvnc-filetransfer/handlefiletransferrequest.c +++ b/libvncserver/tightvnc-filetransfer/handlefiletransferrequest.c @@ -489,12 +489,6 @@ RunFileDownloadThread(void* client) if(rfbWriteExact(cl, fileDownloadMsg.data, fileDownloadMsg.length) < 0) { rfbLog("File [%s]: Method [%s]: Error while writing to socket \n" , __FILE__, __FUNCTION__); - - if(cl != NULL) { - rfbCloseClient(cl); - CloseUndoneFileDownload(cl, rtcp); - } - FreeFileTransferMsg(fileDownloadMsg); return NULL; }