Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#3824 closed enhancement (fixed)

Introduce DllMain callback to DLL binary

Reported by: Mateusz Łoskot Owned by: Mateusz Łoskot
Priority: normal Milestone: 1.8.0
Component: default Version: svn-trunk
Severity: normal Keywords: dll
Cc:

Description

This is a simple patch that introduces DllMain callback function to gdalXY.dll binary on Windows.

The set up and tear down procedures have become non-trivial and in complex applications it may be a challenge to perform this operations properly. The DllMain? callback serves as a central place for this tasks and it's a place where it can be performed properly and safely.

If accepted, I'm ready to commit.

Attachments (3)

add-dllmain-call.patch (1.6 KB) - added by Mateusz Łoskot 9 years ago.
Patch with DllMain? implementation
add-dllmain-call-unregister-only.patch (1.5 KB) - added by Mateusz Łoskot 9 years ago.
Patch with DllMain? implementation which performs clean-up only.
portable-shared-lib-entry-exit-points.patch (3.9 KB) - added by Mateusz Łoskot 9 years ago.
New version of the patch with Windows and Linux portable library entry and exit points. Successfully tested on Linux 64-bit and confirmed memory leaks around driver registrar are eliminated. See TODO comment, perhaps it's portable across other Unix-like systems.

Download all attachments as: .zip

Change History (9)

Changed 9 years ago by Mateusz Łoskot

Attachment: add-dllmain-call.patch added

Patch with DllMain? implementation

comment:1 Changed 9 years ago by warmerdam

Mateusz,

I think the thread detach and process detaach portions are good and I would be supportive of having them in place. I do not think it is appropriate for force driver registration on dll loading. It is up to applications to decide if they want the generic driver registration or custom registration.

I think you should run this proposal past the list for feedback before committing, and restrict this behavior to trunk.

It is unfortunate the behavior will be different on windows and non-windows systems, but I don't think it is a big problem.

comment:2 Changed 9 years ago by Mateusz Łoskot

Frank,

Yes, I agree with you. I wasn't confident about placing the registration in there, though for presentation and to complete the discussion, it's there.

Sure, I will follow-up through the list.

Changed 9 years ago by Mateusz Łoskot

Patch with DllMain? implementation which performs clean-up only.

Changed 9 years ago by Mateusz Łoskot

New version of the patch with Windows and Linux portable library entry and exit points. Successfully tested on Linux 64-bit and confirmed memory leaks around driver registrar are eliminated. See TODO comment, perhaps it's portable across other Unix-like systems.

comment:3 Changed 9 years ago by Mateusz Łoskot

Owner: changed from warmerdam to Mateusz Łoskot
Status: newassigned

In spite of receiving the green light, I haven't committed this solution yet because I'm observing an interesting segmentation fault from one test script. Here is the story:

mloskot@dog:~/dev/gdal/_svn/trunk/autotest/pyscripts$ ./test_gdal_polygonize.py 
/usr/local/lib/python2.6/dist-packages/GDAL-1.7.0-py2.6-linux-x86_64.egg/osgeo/gdal.py:80: DeprecationWarning: gdal.py was placed in a namespace, it is now available as osgeo.gdal
  DeprecationWarning)
  TEST: test_gdal_polygonize_1 ... 0...10...20...30...40...50...60...70...80...90...100 - done.
success
  TEST: test_gdal_polygonize_2 ... success

Test Script: test_gdal_polygonize
Succeeded: 2
Failed:    0 (0 blew exceptions)
Skipped:   0
Expected fail:0

Segmentation fault

Getting backtrace under the gdb:

mloskot@dog:~/dev/gdal/_svn/trunk/autotest/pyscripts$ gdb python
GNU gdb (GDB) 7.2-ubuntu
This GDB was configured as "x86_64-linux-gnu".
(gdb) run ./test_gdal_polygonize.py
Starting program: /usr/bin/python ./test_gdal_polygonize.py
[Thread debugging using libthread_db enabled]
/usr/local/lib/python2.6/dist-packages/GDAL-1.7.0-py2.6-linux-x86_64.egg/osgeo/gdal.py:80: DeprecationWarning: gdal.py was placed in a namespace, it is now available as osgeo.gdal
  DeprecationWarning)
  TEST: test_gdal_polygonize_1 ... 0...10...20...30...40...50...60...70...80...90...100 - done.
success
  TEST: test_gdal_polygonize_2 ... success

Test Script: test_gdal_polygonize
Succeeded: 2
Failed:    0 (0 blew exceptions)
Skipped:   0
Expected fail:0

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff550c036 in std::basic_string<char, std::char_traits<char>, std::allocator<char> >::~basic_string() ()
   from /usr/lib/libstdc++.so.6
(gdb) bt
#0  0x00007ffff550c036 in std::basic_string<char, std::char_traits<char>, std::allocator<char> >::~basic_string() ()
   from /usr/lib/libstdc++.so.6
#1  0x00007ffff5f2df90 in swq_operation::~swq_operation() () from /home/mloskot/dev/gdal/_svn/trunk/gdal/libgdal.so
#2  0x00007ffff5f2de91 in swq_op_registrar::DeInitialize() () from /home/mloskot/dev/gdal/_svn/trunk/gdal/libgdal.so
#3  0x00007ffff5e9471d in OGRCleanupAll () from /home/mloskot/dev/gdal/_svn/trunk/gdal/libgdal.so
#4  0x00007ffff5c9efcc in GDALDestroy() () from /home/mloskot/dev/gdal/_svn/trunk/gdal/libgdal.so
#5  0x00007ffff597ae7f in __do_global_dtors_aux () from /home/mloskot/dev/gdal/_svn/trunk/gdal/libgdal.so
#6  0x0000000000000000 in ?? ()
(gdb) 

comment:4 Changed 9 years ago by Mateusz Łoskot

Partial patch committed to the trunk r21224:

Added DllMain? callback to set-up and tear-down internal GDAL library resources automatically (Ticket #3824). This patch also includes equivalent functionality for Linux using GCC constructor/destructor attributes, however, temporarily disabled due to unresolved segmentation faults on some occasions - see the ticket comments.

comment:5 Changed 9 years ago by Even Rouault

Milestone: 1.8.0
Resolution: fixed
Status: assignedclosed

valgrind was helpful one more time to diagnose the reason for the crash. The issue was that the "static std::vector<swq_operation*> apoOperations" object was freed before the call to swq_op_registrar::DeInitialize?() that tried to free the elements of the array. I've fixed that by changing apoOperations to be a vector* so that it doesn't get automatically destroyed. So GCC support re-enabled in r21235

comment:6 Changed 9 years ago by Mateusz Łoskot

Thanks Even!

Note: See TracTickets for help on using tickets.