JuliaIO/LibSerialPort.jl

EXCEPTION_ACCESS_VIOLATION when listing ports on windows

Closed this issue · 22 comments

Hi,

Trying to list serial ports on Julia 1.4.1 on Windows 10(guest) on a Virtualbox 6.1/Ubuntu 20.04(host) I get the following error message (LIBSERIALPORT_DEBUG environment variable was set).

julia> get_port_list()
sp: sp_list_ports(000000001DE1F1C0) called.
sp: Enumerating ports.
sp: Opening registry key.
sp: Querying registry key value and data sizes.
sp: Iterating over values.
sp: Found port COM3.
sp: sp_get_port_by_name(COM3, 000000001ED2A4B0) called.
sp: Building structure for port COM3.

Please submit a bug report with steps to reproduce this fault, and any error messages that follow (in their entirety). Thanks.
Exception: EXCEPTION_ACCESS_VIOLATION at 0x7ffcbf8607d3 -- RtlGetCurrentServiceSessionId at C:\Windows\SYSTEM32\ntdll.dll (unknown line)
in expression starting at REPL[5]:1
RtlGetCurrentServiceSessionId at C:\Windows\SYSTEM32\ntdll.dll (unknown line)
RtlFreeHeap at C:\Windows\SYSTEM32\ntdll.dll (unknown line)
free at C:\Windows\System32\msvcrt.dll (unknown line)
get_usb_details at /workspace/srcdir/libserialport-0.1.1\windows.c:340 [inlined]
get_port_details at /workspace/srcdir/libserialport-0.1.1\windows.c:468
sp_get_port_by_name at /workspace/srcdir/libserialport-0.1.1\serialport.c:109
list_append at /workspace/srcdir/libserialport-0.1.1\serialport.c:316
list_ports at /workspace/srcdir/libserialport-0.1.1\windows.c:535
sp_list_ports at /workspace/srcdir/libserialport-0.1.1\serialport.c:350
sp_list_ports at C:\Users\User\.julia\packages\LibSerialPort\d8jfU\src\wrap.jl:155 [inlined]
#get_port_list#4 at C:\Users\User\.julia\packages\LibSerialPort\d8jfU\src\high-level-api.jl:138
get_port_list at C:\Users\User\.julia\packages\LibSerialPort\d8jfU\src\high-level-api.jl:138
unknown function (ip: 000000001D62D0B3)
jl_apply at /cygdrive/d/buildbot/worker/package_win64/build/src\julia.h:1700 [inlined]
do_call at /cygdrive/d/buildbot/worker/package_win64/build/src\interpreter.c:369
eval_value at /cygdrive/d/buildbot/worker/package_win64/build/src\interpreter.c:458
eval_stmt_value at /cygdrive/d/buildbot/worker/package_win64/build/src\interpreter.c:409 [inlined]
eval_body at /cygdrive/d/buildbot/worker/package_win64/build/src\interpreter.c:799
jl_interpret_toplevel_thunk at /cygdrive/d/buildbot/worker/package_win64/build/src\interpreter.c:911
jl_toplevel_eval_flex at /cygdrive/d/buildbot/worker/package_win64/build/src\toplevel.c:814
jl_toplevel_eval_flex at /cygdrive/d/buildbot/worker/package_win64/build/src\toplevel.c:764
jl_toplevel_eval at /cygdrive/d/buildbot/worker/package_win64/build/src\toplevel.c:823 [inlined]
jl_toplevel_eval_in at /cygdrive/d/buildbot/worker/package_win64/build/src\toplevel.c:843
eval at .\boot.jl:331
eval_user_input at D:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.4\REPL\src\REPL.jl:86
macro expansion at D:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.4\REPL\src\REPL.jl:118 [inlined]
#26 at .\task.jl:358
jl_apply at /cygdrive/d/buildbot/worker/package_win64/build/src\julia.h:1700 [inlined]
start_task at /cygdrive/d/buildbot/worker/package_win64/build/src\task.c:687
Allocations: 1740597 (Pool: 1740157; Big: 440); GC: 2
PS C:\Users\User>

The same error occurs when I try to do anything else with the, e.g. open.

Any clues on why this is happening?
The c-library libserialport has received many commits since version 1.1.0 was released four years ago, it might be nice to build from master in libserialport_jll ? ( https://sigrok.org/gitweb/?p=libserialport.git;a=shortlog )

Thx

Can you check whether this is resolved by #61?

pkg> add LibSerialPort#ama/fix-read-methods

This has remained open for some time in part due to lack of testing on Windows. Perhaps you can help us here.

I agree that we should update the C library. I'd like to get #61 merged first.

I've tested your branch, but the same segfault occurs.

Looking more at the code that throws I might be bitten by: sigrokproject/libserialport@02c8a14#diff-23ba024365564263bc3bb4665d30db3d as I'm using an USB ACM device...

Thanks for effort!

@andrewadare
I verified that building from libserialport-master solves my problem, and I guess other issues too (#22). If you don't mind I'll propose a PR to Yggdrasil to update the jll, so that extended testing can occur (I only tested on win64 and linux64)

Here is the updated build jll:
https://github.com/laborg/libserialport_jll.jl

And the buildscript:

using BinaryBuilder, Pkg

name = "libserialport"
version = v"1.2.0"

# Collection of sources required to complete build
sources = [
    GitSource("https://github.com/sigrokproject/libserialport.git", 
        "ffbfc5c76ba8100d21d0141478a6c0d761ecfb2f")
]

# Bash recipe for building across all platforms
script = raw"""
cd $WORKSPACE/srcdir/libserialport/
./autogen.sh 
./configure --prefix=${prefix} --build=${MACHTYPE} --host=${target}
make -j${nproc}
make install
"""

# These are the platforms we will build for by default, unless further
# platforms are passed in on the command line
platforms = supported_platforms()

# Disable FreeBSD for now, because hogweed needs alloca()?
platforms = [p for p in platforms if !(typeof(p) <: FreeBSD)]

# The products that we will ensure are always built
products = [
    LibraryProduct("libserialport", :libserialport)
]

# Dependencies that must be installed before this package can be built
dependencies = Dependency[
]

# Build the tarballs, and possibly a `build.jl` as well.
build_tarballs(ARGS, name, version, sources, script, platforms, products, dependencies)

Heres the PR: JuliaPackaging/Yggdrasil#1100

Update: I realized that updating the jll to version 0.1.2 would, with the current compat definition, result in an unwanted update that I can't guarantee to break. So the way forward is to wait until libserialport is released or adjust the compat to be set to "=0.1.1" instead of "0.1.1", and change the General Registry, which I would only do, if it is supported by the people more involved in SerialLibPort.jl.

@laborg thanks for your contribution to update the jll in Yggdrasil. How do I use the new version 0.1.2 in LibSerialPort.jl (at least to test with)? On my PR branch, I tried changing to libserialport_jll = "0.1.2" in Project.toml and updating the package, but I still see

julia> sp_get_package_version_string()
"0.1.1"

@andrewadare The PR hasn't been merged, because this would lead to the problem that every user of LibSerialPort.jll v0.0.4 package would receive the update to the build from libserialport_jll, but that might cause unexpected problems I didn't dare to force on all users. For this to be tested the compat section in Project.toml needs to be changed from "0.1.1" to "=0.1.1", because only the latter definition guarantees LibSerialPort to get exactely 0.1.1 of libserialport, instead of an widely untested build from master. What complicates this is, that the compat section change needs to be manually changed in the General Registry of Julia...

@andrewadare What you could do is: Pkg.dev LibSerialPort.jl and replace the dependency of libserialport_jll to my build https://github.com/laborg/libserialport_jll.jl . With this I get:

julia> sp_get_lib_version_string()
"1:0:1"

The PR hasn't been merged

I'm a bit confused: to me it looks like JuliaPackaging/Yggdrasil#1100 was merged. Are you referring to a different PR?

The PR hasn't been merged

I'm a bit confused: to me it looks like JuliaPackaging/Yggdrasil#1100 was merged. Are you referring to a different PR?

I'm confused too, because yesterday, the PR was cancelled on my request by @giordano immediately after I created it...

I also see

julia> sp_get_lib_version_string()
"1:0:1"

as you do, even though I still have

sp_get_package_version_string()
"0.1.1"

Thanks, I was confused about the split between build recipe, build artifact and the general registry.

@andrewadare As giordano said, you'll get the new libserialport build if you ] add libserialport_jll#master (the version string won't change because its still 0.1.1 on the master).

Here's my environment on which I verified the fixes on Windows and Linux:

(debuglibserialport) pkg> st
Status `~/debuglibserialport/Project.toml`
  [a05a14c7] LibSerialPort v0.3.0
  [220460dc] libserialport_jll v0.1.2+0 #master (https://github.com/JuliaBinaryWrappers/libserialport_jll.jl)

I added master of libserialport_jll as recommended, and see the same package status entry as you.

(the version string won't change because its still 0.1.1 on the master)

This is helpful; I was hung up on the unchanged version number.

All the tests continue to pass for me on the PR branch (#61), and the example scripts work as expected. I guess I was expecting something to break somewhere, but it all works on my system (macOS with a simple loopback device).

Given that this doesn't seem to break anything (yet), and it fixes major Windows issues, it's a shame we can't get this in without the risk of an uncontrolled dependency change.

Remember that compat bounds can be changed retroactively in the register, if there are problems. It doesn't matter what's written in Project.toml

I think we should switch to using a jll from libserialport master, then. I am willing to continue maintaining the Julia wrapper if there are API updates in the future. Depending on life, I may not get to it immediately, but I'll try to stay engaged.

If you believe the new version of libserialport_jll is fine I can just reopen the pull request for the general registry

+1 from me. @laborg is it okay with you to reopen the registry PR?

Yes of course, but I'm not that invested into LibSerialPort.jl, I just want to use it for one specific simple use case on multiple platforms. Do you know of others that could at least test their work on master before the PR is finally merged?

I am confident we can find additional testers. My request is to reopen and merge JuliaRegistries/General#15584, then I'll switch to the v1.2 jll in #61, which we can test before merging to master.

Thanks!

Just verified that the bug is now fixed and LibSerialPort works well on Linux and Windows! Thanks!