Opened 8 months ago

Closed 7 months ago

Last modified 7 months ago

#6809 closed defect (fixed)

Random crashes JPIP in multi-tread environment

Reported by: bugbuster Owned by: warmerdam
Priority: normal Milestone:
Component: default Version: 2.0.2
Severity: normal Keywords:
Cc:

Description

We have run into random crashes when using JPIP images in a multi-thtread environment. After some tedious debug/logging activities( I am definitely not a multi-thread guru :-) ), I found out where the crash comes from. In the JPIP image destructor (JPIPKAKDataset::~JPIPKAKDataset()) :

  • we call the function CPLHTTPCleanup(),
  • in turn, this function calls curl_easy_cleanup() on all Curl handles.

In the JPIPKAKDataset destructor, we should only clean our own handle. I proposed a patch which applies to the file jpipkakdataset.cpp only.

However, I do not feel completely satisfied by this. Since I added a direct call to curl_easy_cleanup(), the JPIP plugin has a direct dependency to libCurl.

Shouldn't it better better to do the following :

  • add a new function CPLHTTPCleanup2(CURL* myHandle) to cpl_http.cpp,
  • use this new function in ~JPIPKAKDataset().

Attachments (2)

patch_jpip_multi_thread.txt (1.4 KB) - added by bugbuster 8 months ago.
patch to jpipkakdataset.cpp
patch_GdalTicket6809.patch (1.1 KB) - added by bugbuster 7 months ago.

Download all attachments as: .zip

Change History (7)

Changed 8 months ago by bugbuster

Attachment: patch_jpip_multi_thread.txt added

patch to jpipkakdataset.cpp

comment:1 Changed 8 months ago by Even Rouault

Have a look at ogr/ogrsf_frmts/plscenes/ogrplscenesdataset.cpp for example and OGRPLScenesDataset::~OGRPLScenesDataset() with the CLOSE_PERSISTENT option passed to CPLHTTPFetch()

comment:2 Changed 7 months ago by bugbuster

Hi Even,

I have been away from office last week and couldn't answer before. Thanks a lot for your hint. I modified my proposed patch to use the more generic CPLHTTPFetch() I/F. As you can see, I didn't create a new attribute to the JPIPKAKDataset class : instead, I used pszTid, which is declared as an attribute but never used.

Changed 7 months ago by bugbuster

Attachment: patch_GdalTicket6809.patch added

comment:3 Changed 7 months ago by Even Rouault

Resolution: fixed
Status: newclosed

In 37431:

JPIPKAK: fix random crashes JPIP in multi-tread environment (fixes #6809)

comment:4 Changed 7 months ago by Even Rouault

I've committed a simpler version of your patch in r37431 that doesn't need pszTid (which no longer exists in trunk nor 2.1 branch). I could only verify it compiles but have no way to check runtime behaviour. So please check.

comment:5 Changed 7 months ago by bugbuster

It does work with your simplified patch. Thanks

Note: See TracTickets for help on using tickets.