Bug 33732

Зачем перевешивать на p8, если ещё нет для Сизифа??

Прошу прощения. Перевешиваю на Сизиф.

Какие-нибудь новости есть по этому тикету?

Задание #229591 ожидает ревью и аппрува.

(В ответ на комментарий №4)
> Задание #229591 ожидает ревью и аппрува.

Спа-си-бо!!

(В ответ на комментарий №4)
> Задание #229591 ожидает ревью и аппрува.

2ldv@: прошу организовать asap, важно.

(In reply to comment #6)
> (В ответ на комментарий №4)
> > Задание #229591 ожидает ревью и аппрува.
> 
> 2ldv@: прошу организовать asap, важно.

2aen: надеюсь, вы понимаете, что это не тема для багзиллы, обсудим во вторник.

(В ответ на комментарий №7)
> (In reply to comment #6)
> > (В ответ на комментарий №4)
> > > Задание #229591 ожидает ревью и аппрува.
> > 
> > 2ldv@: прошу организовать asap, важно.
> 
> 2aen: надеюсь, вы понимаете, что это не тема для багзиллы, обсудим во вторник.
Ok.

(In reply to comment #4)
> Задание #229591 ожидает ревью и аппрува.

Я (бегло) просмотрел коммиты.

У меня возникли такие вопросы:

$ git --no-pager log --oneline gears/sisyphus.. --reverse
210656e94 Move scope_exit helper class into separate header

Помимо переноса, меняется сигнатура одного конструктора. (Смотрел git show -w --color-moved 210656e947f7f6b761ad4976b039716eca24123c ) Было бы проще осознать, что это значит, если бы это изменение было в отдельном коммите с объяснением:

-   explicit scope_exit(const std::function<void()> &&l_function)
+   explicit scope_exit(std::function<void()> &&l_function)


ad4bcbff3 pkgAcqMethod: add Warning function

    Also fix pkgAcqMethod::Log and pkgAcqMethod::Status

Наверное, поправки в существующих функциях лучше сделать в отдельном коммите от добавления новой. (Чтобы легче было cherry-pick-ать.)

715029da2 Port MethodFd class from Debian's Apt

diff --git a/apt/methods/connect.h b/apt/methods/connect.h
index 01d3f6a9f..4b11c719d 100644
--- a/apt/methods/connect.h
+++ b/apt/methods/connect.h

...

 bool Connect(string To,int Port,const char *Service,int DefPort,
-            int &Fd,unsigned long TimeOut,pkgAcqMethod *Owner);
+            std::unique_ptr<MethodFd> &Fd,unsigned long TimeOut,pkgAcqMethod *Owner);

Задался вопросом, не приведёт ли это к изменению ABI. Посмотрел: насколько я понял, это внутренний header, наружу не выставляется. Так что, кажется, можно не менять soname.

2d755fe1e Port UnwrapTLS from Debian's Apt

Этот и предыдущий коммиты затрагивают существующий код, но я пока внимательно не изучал, нет ли ошибок.

bb9d29c1c Add certificate pinning support
0cacbaa6a (HEAD -> dartktemplar-https0, darktemplar@ALT/sisyphus) 0.5.15lorg2-alt64

(In reply to comment #9)
> (In reply to comment #4)
> > Задание #229591 ожидает ревью и аппрува.
> 
> Я (бегло) просмотрел коммиты.
> 
> У меня возникли такие вопросы:
> 
> $ git --no-pager log --oneline gears/sisyphus.. --reverse
> 210656e94 Move scope_exit helper class into separate header
> 
> Помимо переноса, меняется сигнатура одного конструктора. (Смотрел git show -w
> --color-moved 210656e947f7f6b761ad4976b039716eca24123c ) Было бы проще
> осознать, что это значит, если бы это изменение было в отдельном коммите с
> объяснением:
> 
> -   explicit scope_exit(const std::function<void()> &&l_function)
> +   explicit scope_exit(std::function<void()> &&l_function)

Хотелось бы знать, влияет ли это изменение и исправляет ли оно что-то в существующем коде.

(В ответ на комментарий №9)
> (In reply to comment #4)
> > Задание #229591 ожидает ревью и аппрува.
> 
> Я (бегло) просмотрел коммиты.
> 
> У меня возникли такие вопросы:
> 
> $ git --no-pager log --oneline gears/sisyphus.. --reverse
> 210656e94 Move scope_exit helper class into separate header
> 
> Помимо переноса, меняется сигнатура одного конструктора. (Смотрел git show -w
> --color-moved 210656e947f7f6b761ad4976b039716eca24123c ) Было бы проще
> осознать, что это значит, если бы это изменение было в отдельном коммите с
> объяснением:
> 
> -   explicit scope_exit(const std::function<void()> &&l_function)
> +   explicit scope_exit(std::function<void()> &&l_function)
> 
> 

Это изменение исправляет сигнатуру конструктора, использующего move semantics. Добавил это в описание коммита. Не уверен, что стоит это изменение делать отдельно.

> ad4bcbff3 pkgAcqMethod: add Warning function
> 
>     Also fix pkgAcqMethod::Log and pkgAcqMethod::Status
> 
> Наверное, поправки в существующих функциях лучше сделать в отдельном коммите от
> добавления новой. (Чтобы легче было cherry-pick-ать.)
> 

Вынес исправления в отдельный коммит.

> 715029da2 Port MethodFd class from Debian's Apt
> 
> diff --git a/apt/methods/connect.h b/apt/methods/connect.h
> index 01d3f6a9f..4b11c719d 100644
> --- a/apt/methods/connect.h
> +++ b/apt/methods/connect.h
> 
> ...
> 
>  bool Connect(string To,int Port,const char *Service,int DefPort,
> -            int &Fd,unsigned long TimeOut,pkgAcqMethod *Owner);
> +            std::unique_ptr<MethodFd> &Fd,unsigned long TimeOut,pkgAcqMethod
> *Owner);
> 
> Задался вопросом, не приведёт ли это к изменению ABI. Посмотрел: насколько я
> понял, это внутренний header, наружу не выставляется. Так что, кажется, можно
> не менять soname.
> 

Насколько я знаю, это внутренние заголовки, используемые в основном method-ами apt'a.

(In reply to comment #11)
> (В ответ на комментарий №9)
> > (In reply to comment #4)
> > > Задание #229591 ожидает ревью и аппрува.
> > 
> > Я (бегло) просмотрел коммиты.
> > 
> > У меня возникли такие вопросы:
> > 
> > $ git --no-pager log --oneline gears/sisyphus.. --reverse
> > 210656e94 Move scope_exit helper class into separate header
> > 
> > Помимо переноса, меняется сигнатура одного конструктора. (Смотрел git show -w
> > --color-moved 210656e947f7f6b761ad4976b039716eca24123c ) Было бы проще
> > осознать, что это значит, если бы это изменение было в отдельном коммите с
> > объяснением:
> > 
> > -   explicit scope_exit(const std::function<void()> &&l_function)
> > +   explicit scope_exit(std::function<void()> &&l_function)
> > 
> > 
> 
> Это изменение исправляет сигнатуру конструктора, использующего move semantics.
> Добавил это в описание коммита. Не уверен, что стоит это изменение делать
> отдельно.

Спасибо, а на что это изменение всё-таки может повлиять? Делает ли оно лучше что-то для старого кода? Я плохо понимаю все эти дела: в чём разница от того, есть там const или нет?

Пока прочитал просто вот что -- https://en.cppreference.com/w/cpp/language/move_constructor :

A class can have multiple move constructors, e.g. both T::T(const T&&) and T::T(T&&).

(В ответ на комментарий №12)
> Спасибо, а на что это изменение всё-таки может повлиять? Делает ли оно лучше
> что-то для старого кода? Я плохо понимаю все эти дела: в чём разница от того,
> есть там const или нет?
> 
> Пока прочитал просто вот что --
> https://en.cppreference.com/w/cpp/language/move_constructor :
> 
> A class can have multiple move constructors, e.g. both T::T(const T&&) and
> T::T(T&&).

Move semantics - это перемещение содержимого какой-то переменной в другой её инстанс вместо копирования. Обычно небольшая, а иногда - большая, оптимизация производительности. Соответственно, инстанс переменной, откуда данные перемещаются, при таком перемещении обычно изменяется. Чего нельзя сделать с const инстансами. Соответственно, const T&& - практически никогда не нужен (удалось нагуглить указание, что const T&& требовалось при реализации move semantics + initializer lists). Подозреваю, что в данном случае до этого изменения оно просто могло даунгрейдиться до конструктора копирования.

apt-0.5.15lorg2-alt64 -> sisyphus:

Fri May 17 2019 Aleksei Nikiforov <darktemplar@altlinux> 0.5.15lorg2-alt64
- Ported https support from Debian via https method to apt-https package (Closes: #33732).

Спасибо!0

После этого обновления каждый первый запуск hasher-а заканчивается сегфолтом. Следующий запуск уже проходит нормально (если быстро успеть, иначе проблема снова воспроизводится). Выглядит это в логе так:

=8<==============================================
Jun 03 08:13:53 latitude.evg-krsk.dyndns.org kernel: http[10610]: segfault at 0 ip 0000000000406446 sp 00007ffcf8903d50 error 4 in http[404000+a000]
Jun 03 08:13:53 latitude.evg-krsk.dyndns.org kernel: Code: e8 8f e1 ff ff 4c 09 b4 c4 a0 00 00 00 48 8b 7c 24 08 48 8b 07 48 8b 10 48 83 4c 24 20 01 ff d2 48 8b bb e0 02 00 00 41 89 c6 <48> 8b 07 ff 10 41 39 c6 7d 0f 48 8b bb e0 02 00 00 48 8b 07 ff 10
=8<==============================================

трейсбэк падения при этом такой:
=8<==============================================
root@latitude ~ #coredumpctl gdb
           PID: 10610 (http)
           UID: 500 (evg)
           GID: 500 (evg)
        Signal: 11 (SEGV)
     Timestamp: Mon 2019-06-03 08:13:53 +07 (10min ago)
  Command Line: /usr/lib64/apt/methods/http
    Executable: /usr/lib64/apt/methods/http
 Control Group: /user.slice/user-500.slice/session-3.scope
          Unit: session-3.scope
         Slice: user-500.slice
       Session: 3
     Owner UID: 500 (evg)
       Boot ID: 1232570f0b954f0c8e1ba72ea500b151
    Machine ID: 6ecf428dd379f9a7af3889285c1096fb
      Hostname: latitude.evg-krsk.dyndns.org
       Storage: /var/lib/systemd/coredump/core.http.500.1232570f0b954f0c8e1ba72ea500b151.10610.1559524433000000.lz4
       Message: Process 10610 (http) of user 500 dumped core.
                
                Stack trace of thread 10610:
                #0  0x0000000000406446 n/a (http)
                #1  0x0000000000407903 n/a (http)
                #2  0x000000000040ad7c n/a (http)
                #3  0x00000000004053ed n/a (http)
                #4  0x00007fe8464f308b __libc_start_main (libc.so.6)
                #5  0x00000000004055ca n/a (http)

GNU gdb (GDB) 8.2.50.20180917-alt2 (ALT Sisyphus)
Copyright (C) 2018 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-alt-linux".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /usr/lib64/apt/methods/http...
Reading symbols from /usr/lib/debug/usr/lib64/apt/methods/http.debug...
[New LWP 10610]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
Core was generated by `/usr/lib64/apt/methods/http'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  HttpMethod::Go (this=0x7ffcf89040d0, ToFile=<optimized out>, Srv=0x1812150) at /usr/include/c++/8/bits/unique_ptr.h:342
342           get() const noexcept
(gdb) bt full
#0  HttpMethod::Go (this=0x7ffcf89040d0, ToFile=<optimized out>, Srv=0x1812150) at /usr/include/c++/8/bits/unique_ptr.h:342
        rfds = {fds_bits = {1, 0 <repeats 15 times>}}
        wfds = {fds_bits = {8, 0 <repeats 15 times>}}
        FileFD = std::unique_ptr<MethodFd> = {get() = 0x1813250}
        MaxFd = 3
        tv = {tv_sec = 25240944, tv_usec = 25240944}
        Res = <optimized out>
        __d = <optimized out>
        __d = <optimized out>
#1  0x0000000000407903 in ServerState::RunData (this=0x1812150) at http.cc:492
No locals.
#2  0x000000000040ad7c in HttpMethod::Loop (this=0x7ffcf89040d0) at http.cc:1262
        Result = <optimized out>
        UBuf = {actime = 1559518779, modtime = 1559518779}
        Result = <optimized out>
        Res = {MD5Sum = "", SHA1Sum = "", SignatureFP = "", LastModified = 1559518779, IMSHit = false, 
          Filename = "/home/evg/.hasher/lists/partial/ftp.altlinux.org_pub_distributions_ALTLinux_Sisyphus_x86%5f64_base_release.classic", TmpFilename = "", Size = 147, ResumePoint = 0}
        Redirected = std::map with 0 elements
        FailCounter = 0
#3  0x00000000004053ed in main () at http.cc:1408
        Mth = {<pkgAcqMethod> = {_vptr.pkgAcqMethod = 0x40e690 <vtable for HttpMethod+16>, Flags = 6, Messages = std::vector of length 0, capacity 8, Queue = 0x1811ca0, QueueBack = 0x0, 
            FailExtra = ""}, static FailFile = {static npos = 18446744073709551615, 
            _M_dataplus = {<std::allocator<char>> = {<__gnu_cxx::new_allocator<char>> = {<No data fields>}, <No data fields>}, 
              _M_p = 0x1825b00 "/home/evg/.hasher/lists/partial/ftp.altlinux.org_pub_distributions_ALTLinux_Sisyphus_x86%5f64_base_release.classic"}, _M_string_length = 114, {
              _M_local_buf = "r", '\000' <repeats 14 times>, _M_allocated_capacity = 114}}, static FailFd = 3, static FailTime = 1559518779, NextURI = "", 
          AuthList = std::vector of length 0, capacity 0, File = 0x18137a0, Server = 0x1812150}
(gdb) 
=8<==============================================

(В ответ на комментарий №16)
> После этого обновления каждый первый запуск hasher-а заканчивается сегфолтом.
> Следующий запуск уже проходит нормально (если быстро успеть, иначе проблема
> снова воспроизводится). Выглядит это в логе так:

К сожалению, пока что не удалось воспроизвести, а в backtrace из-за оптимизации приложения недостаточно точная и полная информация.

Можете проверить на тестовом задании #231369 воспроизводится ли проблема или же это задание исправляет его?

Если проблема с этим заданием воспроизводится, то я подготовил тестовое задание #231370, где отключена оптимизация при сборке. Можете попробовать воспроизвести проблему с этим заданием, дать backtrace с ним, а также в gdb вывод команды 'print *Srv', и конфигурацию hasher и используемый способ запуска (команду запуска) hasher?

Удалось воспроизвести проблему, изменения из задания #231369 исправляют её. Займусь подготовкой сборки.

apt-0.5.15lorg2-alt66 -> sisyphus:

Wed Jun 05 2019 Aleksei Nikiforov <darktemplar@altlinux> 0.5.15lorg2-alt66
- Fortified https method (Closes: #33732)
- Dropped processing Realm name in http/https methods (Closes: #33236)