Project

General

Profile

Bug #12004

Changing lens when in tethering mode makes DT crash

Added by michael rasmussen 10 months ago. Updated 8 months ago.

Status:
Fixed
Priority:
Low
Assignee:
-
Category:
Tethering
Target version:
Start date:
02/09/2018
Due date:
% Done:

100%

Affected Version:
2.4.1
System:
Debian
bitness:
64-bit
hardware architecture:
amd64/x86

Description

If in an active tethering session and you want to change lens the as soon as the camera detects that the lens has been removed Darktable will crash with a segmentation fault.

Camera: Nikon D600
Lens (in this report): AF-S Nikkor 24-85 ED f/3.5-4.5 G
How often does it happen: Every time

darktable_bt_Q5WTDZ.txt Magnifier - backtrace of crash (darktable -d input) (60.9 KB) michael rasmussen, 02/09/2018 03:12 AM

darktable_bt_7V53DZ.txt Magnifier (66.3 KB) michael rasmussen, 02/10/2018 01:30 PM

darktable_bt_VQO1DZ.txt Magnifier (66 KB) michael rasmussen, 02/10/2018 03:34 PM

darktable_bt_N6NXDZ.txt Magnifier (71.7 KB) michael rasmussen, 02/10/2018 05:35 PM

darktable_bt_7BV0DZ.txt Magnifier (67.4 KB) michael rasmussen, 02/10/2018 05:49 PM

darktable_bt_XH07DZ.txt Magnifier (67.4 KB) michael rasmussen, 02/10/2018 06:31 PM

fixes_bug_12004.patch Magnifier (881 Bytes) michael rasmussen, 02/10/2018 09:12 PM

fixes_bug_12004_correct_code_style.patch Magnifier (884 Bytes) michael rasmussen, 02/10/2018 09:33 PM

gphoto2_all_config.txt Magnifier (56 KB) michael rasmussen, 02/10/2018 09:54 PM

cam.diff Magnifier (21.2 KB) Tobias Ellinghaus, 02/10/2018 10:32 PM

darktable_bt_AJLNDZ.txt Magnifier (63.4 KB) michael rasmussen, 02/10/2018 10:55 PM

fixes_bug_12004_v2.patch Magnifier (921 Bytes) michael rasmussen, 02/11/2018 12:27 AM

Associated revisions

Revision 49def0e2
Added by Tobias Ellinghaus 10 months ago

Tethering: Add some error handling

This is related to bug #12004.

Revision 19407a59
Added by Tobias Ellinghaus 10 months ago

Tethering: More error handling

Wtf is this code trying to do?

See issue #12004.

Revision acfcdc4d
Added by Tobias Ellinghaus 10 months ago

Tethering: Try to fix a crash when changing lenses

This is related to bug #12004.

Revision 122a659c
Added by Tobias Ellinghaus 10 months ago

Tethering: Add some error handling

This is related to bug #12004.

(cherry picked from commit 49def0e24fc3118eb9ced5d64154a19868723302)

Revision 2a3f8eee
Added by Tobias Ellinghaus 10 months ago

Tethering: More error handling

Wtf is this code trying to do?

See issue #12004.

(cherry picked from commit 19407a5926008c787440c7780fe394575477b343)

Revision 31f90c8a
Added by Tobias Ellinghaus 10 months ago

Tethering: Try to fix a crash when changing lenses

This is related to bug #12004.

(cherry picked from commit acfcdc4d7731657c83d9d0ae0d987c727ed87f33)

History

#1 Updated by michael rasmussen 10 months ago

Sorry, affected version was wrong. Correct version: darktable/unstable,now 2.4.0-1+b1

#2 Updated by Tobias Ellinghaus 10 months ago

It seems you are missing the debug symbols for darktable. Please install darktable-dbgsym and get a new backtrace. You might have to add debian-debug to the apt sources first.

#3 Updated by michael rasmussen 10 months ago

Tobias Ellinghaus wrote:

It seems you are missing the debug symbols for darktable. Please install darktable-dbgsym and get a new backtrace. You might have to add debian-debug to the apt sources first.

darktable-dbgsym is not currently available in Debian Unstable. See https://packages.debian.org/sid/darktable-dbgsym

#4 Updated by Tobias Ellinghaus 10 months ago

That's why I told you to add the debug repo first.

$ LANG=C apt-cache policy darktable-dbgsym 
darktable-dbgsym:
  Installed: (none)
  Candidate: 2.4.0-1+b1
  Version table:
     2.4.0-1+b1 500
        500 http://ftp.de.debian.org/debian-debug unstable-debug/main amd64 Packages

#5 Updated by michael rasmussen 10 months ago

Tobias Ellinghaus wrote:

That's why I told you to add the debug repo first.

[...]

I was not aware of the new policy of having debug symbols in a separate repository.

Crash file added with debug symbols installed

dpkg -s darktable-dbgsym
Package: darktable-dbgsym
Status: install ok installed
Priority: optional
Section: debug
Installed-Size: 22372
Maintainer: Debian PhotoTools Maintainers <>
Architecture: amd64
Source: darktable (2.4.0-1)
Version: 2.4.0-1+b1
Depends: darktable (= 2.4.0-1+b1)
Description: debug symbols for darktable

#6 Updated by Tobias Ellinghaus 10 months ago

Thanks, that's much better already. :-)

I wasn't able to reproduce the crash, but I added some error handling to the code where the crash happened. If you are able to compile darktable yourself you can try it.

#7 Updated by michael rasmussen 10 months ago

Tobias Ellinghaus wrote:

Thanks, that's much better already. :-)

I wasn't able to reproduce the crash, but I added some error handling to the code where the crash happened. If you are able to compile darktable yourself you can try it.

Which branch should I build from?

#8 Updated by michael rasmussen 10 months ago

michael rasmussen wrote:

Tobias Ellinghaus wrote:

Thanks, that's much better already. :-)

I wasn't able to reproduce the crash, but I added some error handling to the code where the crash happened. If you are able to compile darktable yourself you can try it.

Which branch should I build from?

Haven problems building?
Run Build Command:"/usr/bin/make" "cmTC_6efe3/fast"
/usr/bin/make -f CMakeFiles/cmTC_6efe3.dir/build.make CMakeFiles/cmTC_6efe3.dir/build
make1: Entering directory '/home/mir/git/darktable/build/CMakeFiles/CMakeTmp'
Building C object CMakeFiles/cmTC_6efe3.dir/src.c.o
/usr/bin/cc -Wall -fno-strict-aliasing -Wformat -Wformat-security -Wshadow -Wtype-limits -Wvla -Wold-style-declaration -DC_COMPILER_UNDERSTANDS_-Wthread-safety -Wall -fno-strict-aliasing -Wformat -Wformat-security -Wshadow -Wtype-limits -Wvla -Wold-style-declaration -Wthread-safety -o CMakeFiles/cmTC_6efe3.dir/src.c.o -c /home/mir/git/darktable/build/CMakeFiles/CMakeTmp/src.c
cc: error: unrecognized command line option '-Wthread-safety'; did you mean '-fthread-jumps'?
CMakeFiles/cmTC_6efe3.dir/build.make:65: recipe for target 'CMakeFiles/cmTC_6efe3.dir/src.c.o' failed
make1: *** [CMakeFiles/cmTC_6efe3.dir/src.c.o] Error 1

#9 Updated by michael rasmussen 10 months ago

michael rasmussen wrote:

michael rasmussen wrote:

Tobias Ellinghaus wrote:

Thanks, that's much better already. :-)

I wasn't able to reproduce the crash, but I added some error handling to the code where the crash happened. If you are able to compile darktable yourself you can try it.

Which branch should I build from?

Haven problems building?
Run Build Command:"/usr/bin/make" "cmTC_6efe3/fast"
/usr/bin/make -f CMakeFiles/cmTC_6efe3.dir/build.make CMakeFiles/cmTC_6efe3.dir/build
make1: Entering directory '/home/mir/git/darktable/build/CMakeFiles/CMakeTmp'
Building C object CMakeFiles/cmTC_6efe3.dir/src.c.o
/usr/bin/cc -Wall -fno-strict-aliasing -Wformat -Wformat-security -Wshadow -Wtype-limits -Wvla -Wold-style-declaration -DC_COMPILER_UNDERSTANDS_-Wthread-safety -Wall -fno-strict-aliasing -Wformat -Wformat-security -Wshadow -Wtype-limits -Wvla -Wold-style-declaration -Wthread-safety -o CMakeFiles/cmTC_6efe3.dir/src.c.o -c /home/mir/git/darktable/build/CMakeFiles/CMakeTmp/src.c
cc: error: unrecognized command line option '-Wthread-safety'; did you mean '-fthread-jumps'?
CMakeFiles/cmTC_6efe3.dir/build.make:65: recipe for target 'CMakeFiles/cmTC_6efe3.dir/src.c.o' failed
make1: *** [CMakeFiles/cmTC_6efe3.dir/src.c.o] Error 1

sudo apt-get build-dep darktable seems to have fixed it.

#10 Updated by michael rasmussen 10 months ago

michael rasmussen wrote:

michael rasmussen wrote:

michael rasmussen wrote:

Tobias Ellinghaus wrote:

Thanks, that's much better already. :-)

I wasn't able to reproduce the crash, but I added some error handling to the code where the crash happened. If you are able to compile darktable yourself you can try it.

Which branch should I build from?

Haven problems building?
Run Build Command:"/usr/bin/make" "cmTC_6efe3/fast"
/usr/bin/make -f CMakeFiles/cmTC_6efe3.dir/build.make CMakeFiles/cmTC_6efe3.dir/build
make1: Entering directory '/home/mir/git/darktable/build/CMakeFiles/CMakeTmp'
Building C object CMakeFiles/cmTC_6efe3.dir/src.c.o
/usr/bin/cc -Wall -fno-strict-aliasing -Wformat -Wformat-security -Wshadow -Wtype-limits -Wvla -Wold-style-declaration -DC_COMPILER_UNDERSTANDS_-Wthread-safety -Wall -fno-strict-aliasing -Wformat -Wformat-security -Wshadow -Wtype-limits -Wvla -Wold-style-declaration -Wthread-safety -o CMakeFiles/cmTC_6efe3.dir/src.c.o -c /home/mir/git/darktable/build/CMakeFiles/CMakeTmp/src.c
cc: error: unrecognized command line option '-Wthread-safety'; did you mean '-fthread-jumps'?
CMakeFiles/cmTC_6efe3.dir/build.make:65: recipe for target 'CMakeFiles/cmTC_6efe3.dir/src.c.o' failed
make1: *** [CMakeFiles/cmTC_6efe3.dir/src.c.o] Error 1

sudo apt-get build-dep darktable seems to have fixed it.

This needs to be added as well:
sudo apt install llvm-6.0 clang-6.0 python3-jsonschema

#11 Updated by michael rasmussen 10 months ago

Tobias Ellinghaus wrote:

Thanks, that's much better already. :-)

I wasn't able to reproduce the crash, but I added some error handling to the code where the crash happened. If you are able to compile darktable yourself you can try it.

Added crash file from latest build from master:
/opt/darktable/bin/darktable --version
this is darktable 2.5.0+218~g41cf89ff4
copyright (c) 2009-2018 johannes hanika

compile options:
bit depth is 64 bit
normal build
SSE2 optimized codepath enabled
OpenMP support enabled
OpenCL support enabled
Lua support enabled, API version 5.0.0
Colord support enabled
gPhoto2 support enabled
GraphicsMagick support enabled
OpenEXR support enabled

#12 Updated by Tobias Ellinghaus 10 months ago

Oh, I forgot to actually push the commit. Sorry for that. Please try again.

About llvm-6.0, clang-6.0 and python3-jsonschema, those are not required.

You can also use the snapshot builds darix makes if you don't feel like compiling yourself:

https://software.opensuse.org/download.html?project=graphics:darktable:master&package=darktable

#13 Updated by michael rasmussen 10 months ago

Tobias Ellinghaus wrote:

Oh, I forgot to actually push the commit. Sorry for that. Please try again.

Apparently an unhandled unknown event results in a null pointer?
#8 0x00007fcc902d0d5c in _camera_poll_events (c=c@entry=0x5588cef46120, cam=cam@entry=0x5588cf5a5bf0) at /home/mir/git/darktable/src/common/camera_control.c:1463
event = GP_EVENT_UNKNOWN
data = 0x7fcbd06f3a50

#14 Updated by michael rasmussen 10 months ago

michael rasmussen wrote:

Tobias Ellinghaus wrote:

Oh, I forgot to actually push the commit. Sorry for that. Please try again.

Apparently an unhandled unknown event results in a null pointer?
#8 0x00007fcc902d0d5c in _camera_poll_events (c=c@entry=0x5588cef46120, cam=cam@entry=0x5588cf5a5bf0) at /home/mir/git/darktable/src/common/camera_control.c:1463
event = GP_EVENT_UNKNOWN
data = 0x7fcbd06f3a50

Please see attached file produced with running: darktable -d camctl
If I interpret the debug output correct then this part is not activated:
if(event == GP_EVENT_UNKNOWN) {
/* this is really some undefined behavior, seems like its
camera driver dependent... very ugly! */
if(strstr((char *)data, "4006") || // Nikon PTP driver
(strstr((char *)data, "PTP Property")
&& strstr((char *)data, "changed")) // Some Canon driver maybe all ??
) {
// Property change event occurred on camera
// let's update cache and signalling
dt_print(DT_DEBUG_CAMCTL, "[camera_control] Camera configuration change event, lets update internal "
"configuration cache.\n");
_camera_configuration_update(c, cam);
}
#4 0x00007f5e10f42c05 in _camera_configuration_merge (c=c@entry=0x557550963b70, camera=camera@entry=0x7f5dd80099a0, source=<optimized out>, destination=destination@entry=0x7f5dd802b1d0, notify_all=notify_all@entry=0) at /home/mir/git/darktable/src/common/camera_control.c:1560
res = <optimized out>
children = <optimized out>
sk = 0x7f5d560f1074 "5008" <--- Only catch: strstr((char *)data, "4006"
stv = 0x7f5d560f1200 "0"
dw = 0x7f5dd807e020
dtv = 0x45160000 <error: Cannot access memory at address 0x45160000>
type = GP_WIDGET_MENU

#15 Updated by Tobias Ellinghaus 10 months ago

No idea what's going on there. It's strange code. Please pull again.

#16 Updated by michael rasmussen 10 months ago

Tobias Ellinghaus wrote:

No idea what's going on there. It's strange code. Please pull again.

Same story;-(

Forgot to write that last debug info written to console before the crash is:
[camera_control] Camera configuration change event, lets update internal configuration cache.

An idea: Instead of updating internal configuration cache when this event occurs simply clean internal configuration cache and wait for mount event of new lens to occur. When mount occurs simply rebuild internal configuration cache?

#17 Updated by michael rasmussen 10 months ago

michael rasmussen wrote:

Tobias Ellinghaus wrote:

No idea what's going on there. It's strange code. Please pull again.

Same story;-(

Forgot to write that last debug info written to console before the crash is:
[camera_control] Camera configuration change event, lets update internal configuration cache.

An idea: Instead of updating internal configuration cache when this event occurs simply clean internal configuration cache and wait for mount event of new lens to occur. When mount occurs simply rebuild internal configuration cache?

Just tried starting tethering mode without any lens attached to the body and attaching a lens when in tethering mode. No crash, so maybe delete internal configuration cache when event code 5008 is seen is all that is required to handle this situation?

#18 Updated by michael rasmussen 10 months ago

Hi Tobias,

I seemed to have a patch which fixes this bug. However, I do not know the darktable code down to its smallest detail in which case I am not able to know whether the patch has side effects which breaks other things. So please evaluate the patch.

#19 Updated by michael rasmussen 10 months ago

michael rasmussen wrote:

Hi Tobias,

I seemed to have a patch which fixes this bug. However, I do not know the darktable code down to its smallest detail in which case I am not able to know whether the patch has side effects which breaks other things. So please evaluate the patch.

Patch obeying the darktable coding style;-)

#20 Updated by Tobias Ellinghaus 10 months ago

Does it also crash when removing the lens and then re-attaching the same lens?

Could you also show the output of gphoto2 --list-all-config

I am not sure if that patch is going to work, it's not clear to me if the flag is ever set back to false, so eventually we will still run into the crash.

#21 Updated by michael rasmussen 10 months ago

Tobias Ellinghaus wrote:

Does it also crash when removing the lens and then re-attaching the same lens?

This test is impossible to run since DT crashes as soon as the lens is removed but as written above you can start DT without a lens attached and the attached it after goind into tethering mode without DT crashing.

Could you also show the output of gphoto2 --list-all-config

Rather big so attached as file.

I am not sure if that patch is going to work, it's not clear to me if the flag is ever set back to false, so eventually we will still run into the crash.

According to http://www.gphoto.org/doc/api/gphoto2-widget_8h.html#adaf6cc55afadae959d07adebecc85c65
"

Tells if the widget has been changed.

Parameters
widget a CameraWidget

Returns
a gphoto2 error code or changed flag.

Returns 1 if the state of the CameraWidget has been changed or 0 if not. In addition, it resets the changed flag to 0."
So flag is reset at every call to the helper function.

#22 Updated by Tobias Ellinghaus 10 months ago

That behaviour was recently changed, the state is no longer reset to 0 in gp_widget_changed.

I added some debug output, maybe that can help us find where/why it's crashing. To me it looks a bit like an unrelated memory corruption. Please apply the attached patch and see what you get.

#23 Updated by michael rasmussen 10 months ago

Tobias Ellinghaus wrote:

That behaviour was recently changed, the state is no longer reset to 0 in gp_widget_changed.

I added some debug output, maybe that can help us find where/why it's crashing. To me it looks a bit like an unrelated memory corruption. Please apply the attached patch and see what you get.

New crash file attached.

#24 Updated by michael rasmussen 10 months ago

Tobias Ellinghaus wrote:

That behaviour was recently changed, the state is no longer reset to 0 in gp_widget_changed.

So in the future you would need to call this explicitly?
gp_widget_set_changed(widget, 0)

#25 Updated by Tobias Ellinghaus 10 months ago

That backtrace makes no sense. According to it "5008" is the one where it crashes, and that is of type GP_WIDGET_MENU. However, according to the gphoto2 output it should be type RANGE. Do you maybe have more than one copy of libgphoto2 installed in parallel? What's the output of dpkg -l | grep gphoto2?

#26 Updated by michael rasmussen 10 months ago

Tobias Ellinghaus wrote:

That backtrace makes no sense. According to it "5008" is the one where it crashes, and that is of type GP_WIDGET_MENU. However, according to the gphoto2 output it should be type RANGE. Do you maybe have more than one copy of libgphoto2 installed in parallel? What's the output of dpkg -l | grep gphoto2?

dpkg -l | grep gphoto2
ii gphoto2 2.5.15-1 amd64 digital camera command-line client
ii libgphoto2-6:amd64 2.5.16-2 amd64 gphoto2 digital camera library
ii libgphoto2-dev:amd64 2.5.16-2 amd64 gphoto2 digital camera library (development files)
ii libgphoto2-l10n 2.5.16-2 all gphoto2 digital camera library - localized messages
ii libgphoto2-port12:amd64 2.5.16-2 amd64 gphoto2 digital camera port library

#27 Updated by Tobias Ellinghaus 10 months ago

  • % Done changed from 0 to 20
  • Status changed from New to Incomplete

That looks sane, too. I am running out of ideas. I will ping the gphoto2 guy and see if he has any ideas. It might take a few days, not sure if he's coming online over the weekend.

#28 Updated by michael rasmussen 10 months ago

Tobias Ellinghaus wrote:

That looks sane, too. I am running out of ideas. I will ping the gphoto2 guy and see if he has any ideas. It might take a few days, not sure if he's coming online over the weekend.

Will be interesting to hear his opinion.

Meanwhile, this patch works great ;-)

#29 Updated by Tobias Ellinghaus 10 months ago

I had a chat with Marcus. It seems that libgphoto2 has a feature that sometimes turns a RANGE into a MENU. When detaching the lens that happens. That can lead to the situation that a pointer to a float is treated as a char pointer and darktable tries to compare the strings. I tried to work around the problem, please test.

Querying the lens data 3 times, in the 2nd case the lens is detached:

gphoto2: {...ibgphoto2/camlibs/ptp2} /> get-config 5008
Label: Focal Length
Readonly: 1
Type: RANGE
Current: 2400
Bottom: 2400
Top: 12000
Step: 1
END
gphoto2: {...ibgphoto2/camlibs/ptp2} /> get-config 5008
Label: Focal Length
Readonly: 1
Type: MENU
Current: 0
Choice: 0 0
END
gphoto2: {...ibgphoto2/camlibs/ptp2} /> get-config 5008
Label: Focal Length
Readonly: 1
Type: RANGE
Current: 2400
Bottom: 2400
Top: 12000
Step: 1
END

#30 Updated by michael rasmussen 10 months ago

Tobias Ellinghaus wrote:

I had a chat with Marcus. It seems that libgphoto2 has a feature that sometimes turns a RANGE into a MENU. When detaching the lens that happens. That can lead to the situation that a pointer to a float is treated as a char pointer and darktable tries to compare the strings. I tried to work around the problem, please test.

Querying the lens data 3 times, in the 2nd case the lens is detached:

[...]

Hi Tobias,

Your patch seems to have fixed this bug. Nice patch by the way;-)
To confirm I tried the following:
1) Initiate tethering mode with lens attached and then detached lens -> Ok
2) Initiate tethering mode with lens detached and then attached lens -> Ok
3) Initiate tethering mode with lens attached, then detached lens and attach same lens again -> Ok
4) Initiate tethering mode with lens attached, then detached lens and attach another lens -> Ok

In all tests above when shooting the new image was displayed in the editor.

A question: Will the patch be backported to the 2.4.x series of Darktable?

#31 Updated by Tobias Ellinghaus 10 months ago

  • % Done changed from 20 to 100
  • Status changed from Incomplete to Fixed

There are some more small changes I want to do, but once that is done it will be backported, yes.

#32 Updated by Roman Lebedev 8 months ago

  • Target version set to 2.6.0

Also available in: Atom PDF