10 октября 2018 года наша команда выпустила новую версию приложения на React Native. Мы рады и гордимся этим.

Но ужас-то какой: через несколько часов внезапно увеличивается количество сбоев под Android.


10 000 сбоев под Android

Наш инструмент мониторинга сбоев Sentry сходит с ума.

Во всех случаях мы видим ошибку типа JSApplicationIllegalArgumentException Error while updating property 'left' in shadow node of type: RCTView".

В React Native это обычно происходит, если задать свойство с неправильным типом. Но почему ошибка не проявилась при тестировании? У нас каждый разработчик тщательно тестирует новые релизы на нескольких устройствах.

Также ошибки кажутся довольно случайными, они как будто выпадают на любой комбинации свойств и типа shadow ноды. Например, вот первые три:

  • Error while updating property 'paddingTop' in shadow node of type: RCTView
  • Error while updating property 'height' in shadow node of type: RCTImageView
  • Error while updating property 'fill' of a view managed by: RNSVGPath

Похоже, ошибка происходит на любом устройстве и в любой версии Android, судя по отчету Sentry.


Большинство сбоев под Android 8.0.0 падает, но это согласуется с нашей пользовательской базой

Давайте воспроизведём!


Итак, первый шаг перед исправлением бага — воспроизвести его, верно? К счастью, благодаря логам Sentry, мы можем узнать, что делают пользователи перед появлением сбоя.

Та-а-ак, посмотрим…



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

Хорошо, попробуем повторить. Устанавливаем приложение на шести устройствах Android, открываем его и выходим несколько раз. Нет сбоя! Тем более невозможно воспроизвести его локально в dev-режиме.

Ладно, это кажется бессмысленным. Сбои всё равно довольно случайны и происходят в 10% случаев. Похоже, что у вас шанс 1 из 10, что приложение упадёт при запуске.

Анализ трассировки стека


Чтобы воспроизвести этот сбой, давайте попробуем понять, откуда он берётся…


Как упоминалось ранее, у нас несколько различных ошибок. И у всех похожие, но немного разные трассировки.

Хорошо, возьмём первую из них:

java.lang.ArrayIndexOutOfBoundsException: length=10; index=-1
    at android.support.v4.util.Pools$SimplePool.release(Pools.java:116)
    at com.facebook.react.bridge.DynamicFromMap.recycle(DynamicFromMap.java:40)
    at com.facebook.react.uimanager.LayoutShadowNode.setHeight(LayoutShadowNode.java:168)
    at java.lang.reflect.Method.invoke(Method.java)
    ...

java.lang.reflect.InvocationTargetException: null
    at java.lang.reflect.Method.invoke(Method.java)
    ...

com.facebook.react.bridge.JSApplicationIllegalArgumentException: Error while updating property 'height' in shadow node of type: RNSVGSvgView
    at com.facebook.react.uimanager.ViewManagersPropertyCache$PropSetter.updateShadowNodeProp(ViewManagersPropertyCache.java:113)
    ...

Итак, проблема в android/support/v4/util/Pools.java.

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

Найдём другой способ


Другой способ найти основную причину ошибки — проверить новые изменения, внесённые в последний релиз. Особенно те, которые влияют на нативный код Android. Возникают две гипотезы:

  • Мы обновили Native Navigation, где для каждого экрана используются нативные фрагменты под Android.
  • Мы обновили react-native-svg. Было несколько исключений, связанных с компонентами SVG, но вряд ли дело в этом.

На данный момент мы не можем воспроизвести ошибку, поэтому лучшая стратегия:

  1. Откатить обратно одну из двух библиотек.Выкатить её для 10% пользователей, что тривиально делается в Play Store.Проверить у нескольких пользователей, сохранился ли сбой. Таким образом мы подтвердим или опровергнем гипотезу.


    Но как выбрать библиотеку для отката? Конечно, можно подбросить монетку, но разве это оптимальный вариант?


    Добираемся до сути


    Давайте более тщательно проанализируем предыдущую трассировку. Возможно, это поможет определиться с библиотекой.

    /**
     * Simple (non-synchronized) pool of objects.
     *
     * @param  The pooled type.
     */
    public static class SimplePool implements Pool {
        private final Object[] mPool;
    
        private int mPoolSize;
    
        ...
    
        @Override
        public boolean release(T instance) {
            if (isInPool(instance)) {
                throw new IllegalStateException("Already in the pool!");
            }
            if (mPoolSize < mPool.length) {
                mPool[mPoolSize] = instance;
                mPoolSize++;
                return true;
            }
            return false;
        }

    Здесь произошёл сбой. Ошибка java.lang.ArrayIndexOutOfBoundsException: length=10; index=-1 означает, что mPool — массив размером 10, но mPoolSize=-1.

    Ладно, как получился mPoolSize=-1? Кроме метода recycle выше, единственным местом изменения mPoolSize является метод acquire класса SimplePool:

    public T acquire() {
        if (mPoolSize > 0) {
            final int lastPooledIndex = mPoolSize - 1;
            T instance = (T) mPool[lastPooledIndex];
            mPool[lastPooledIndex] = null;
            mPoolSize--;
            return instance;
        }
        return null;
    }

    Поэтому единственный способ получить отрицательное значение mPoolSize — это уменьшить его при mPoolSize=0. Но как это возможно с условием mPoolSize > 0?

    Поставим точки останова в Android Studio и по шагам посмотрим, что происходит при запуске приложения. Я имею в виду, здесь условие if, этот код должен нормально работать!

    Наконец, откровение!



    Смотрите, в DynamicFromMap статическая ссылка на SimplePool.

    private static final Pools.SimplePool<DynamicFromMap> sPool = new Pools.SimplePool<>(10);

    После нескольких десятков нажатий кнопки Play с тщательно расставленными точками останова мы видим, что потоки mqt_native_modules обращаются к функциям SimplePool.acquire и SimplePool.release с помощью React Native для управления свойствами стиля компонента React (ниже свойство width компонента)



    Но к ним также обращается основной поток main!



    Выше мы видим, что они используются для обновления свойства fill в основном потоке, как правило, для компонента react-native-svg! И действительно, библиотека react-native-svg начала использовать DynamicFromMap только с седьмой версии для улучшения производительности нативных svg-анимаций.

    И-и-и… функцию можно вызвать из двух потоков, но DynamicFromMap не использует SimplePool потокобезопасным способом. «Потокобезопасным», говорите?

    Потокобезопасность, немного теории


    В однопоточном JavaScript разработчикам обычно не нужно иметь дело с потокобезопасностью.

    С другой стороны, Java поддерживает концепцию параллельных или многопоточных программ. Несколько потоков могут выполняться в рамках одной программы и потенциально могут обращаться к общей структуре данных, что порой приводит к неожиданным результатам.

    Возьмём простой пример: на изображении ниже показано, что потоки A и B параллельно:

    • считывают целое число;
    • увеличивают его значение;
    • возвращают его.


    Поток B потенциально может получить доступ к значению данных до того, как поток A его обновит. Мы ожидали, что два отдельных шага дадут конечное значение 19. Вместо этого мы можем получить 18. Такая ситуация, где конечное состояние данных зависит от относительного порядка операций потока, называется состоянием гонки. Проблема в том, что это состояние необязательно возникает всё время. Возможно, в приведённом выше случае у потока B есть другая работа, прежде чем приступить к увеличению значения, что даёт достаточно времени потоку A для обновления значения. Это объясняет случайность и невозможность воспроизвести сбой.

    Структура данных считается потокобезопасной, если операции могут выполняться одновременно несколькими потоками без риска возникновения состояния гонки.

    Когда один поток выполняет чтение для определённого элемента данных, другой поток не должен иметь права изменять или удалять этот элемент (это называется атомарностью). В предыдущем примере, если бы циклы обновления были атомарными, условия гонки можно было бы избежать. Поток B будет ждать, пока поток A завершит операцию, и затем начинает сам.

    В нашем случае может произойти вот что:



    Поскольку DynamicFromMap содержит статическую ссылку на SimplePool, несколько вызовов DynamicFromMap поступают из разных потоков, одновременно вызывая метод acquire в SimplePool.

    На иллюстрации выше поток A вызывает метод, оценивая условие как true, но он ещё не успел уменьшить значение mPoolSize (которое используется совместно с потоком B), в то время как поток B тоже вызывает этот метод и тоже оценивает условие как true. Впоследствии каждый вызов уменьшит значение mPoolSize, в результате чего и получается «невозможное» значение.

    Исправление


    Изучая варианты исправления, мы обнаружили пул-реквест на react-native, который ещё не влился в ветку — и он обеспечивает потокобезопасность в данном случае.



    Затем мы выкатили исправленную версию React Native для пользователей. Сбой наконец-то исправлен, ура!


    Итак, благодаря помощи Дженика Дюплесси (контрибьютор в ядро React Native) и Микаэля Сэнда (мейнтейнер react-native-svg), патч включён в следующую минорную версию React Native 0.57.

    Исправление этого бага потребовало некоторых усилий, но это была отличная возможность глубже покопаться в react-native и react-native-svg. Хороший отладчик и несколько хорошо расположенных точек останова имеют большое значение. Надеюсь, вы тоже вынесли из этой истории что-то полезное!

Комментарии (22)


  1. alex_zzzz
    07.12.2018 17:45
    +5

    На данный момент мы не можем воспроизвести ошибку, поэтому лучшая стратегия:
    Откатить обратно одну из двух библиотек.Выкатить её для 10% пользователей, что тривиально делается в Play Store.Проверить у нескольких пользователей, сохранился ли сбой. Таким образом мы подтвердим или опровергнем гипотезу.

    Интересно, чем плоха была стратегия откатить бедных пользователей с проблемного билда на нормальный и разбираться со своими проблемами самостоятельно.


    1. Javian
      07.12.2018 17:52

      офф похоже там работают серийные программисты habr.com/post/309388


    1. Drag13
      07.12.2018 18:14

      Ну как, «откатить». Можно только выпустить новую версию либы и попросить людей обновиться самостоятельно. Всех это не спасет, но хоть что-то. А тут вообще нехорошо получилось.


    1. mkshma
      07.12.2018 18:35
      +8

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


    1. zagayevskiy
      07.12.2018 20:24
      +2

      А что, вы знаете простой способ это сделать?


      1. Hardcoin
        07.12.2018 22:14

        Нормальный способ выложить свежую версию, которая по факту будет предыдущей? Это тривиально — способ ровно такой, каким они выложили эту бажную.


        1. zagayevskiy
          07.12.2018 22:47
          +2

          Ну-ну, ещё не забыть правильно всё откатить. Ещё бывают либы, которые не умеют. даунгредиться.


          1. Hardcoin
            07.12.2018 22:58

            Возможно я что-то не знаю про разработку под Андроид. Я полагал, что пакет приложения заменяется целиком, это не так?


            Вопрос же был не как вернусь из репозитория версию до обновления (это уж точно не должно вызвать сложностей), а как обновить у пользователей? Так вот если они предыдущую версию зальют (поставив ей версию +1) — Гугл не может это залить пользователям? У них не заведётся?


            1. zagayevskiy
              08.12.2018 01:16
              +2

              Апк-то заменится. А вот пользовательские данные, в общем случае, нужно будет откатывать руками. И не всегда возможно контролировать все данные, которые записали в бд/преференсы все зависимости. Вы рассматриваете самый простой случай, а в жизни всё сложнее, и если не закладываться на даунгрейд(а я не знаю, кто в мобильной разработке на него закладывается), то откатить будет сложно, и багов на этом можно огрести гораздо больше.


              1. Hardcoin
                08.12.2018 11:08

                Зависит от библиотек, конечно, согласен. Если библиотека потребовала изменения формата внутреннего хранения (и при обновлении, например, формат сконвертировался), то это сложности. Или менеджер потребовал какую-то фичу внедрить в тот же релиз.


                Про бакэнд разработку скажу, что изменения должны быть неразрушающие по возможности. Добавили колонки, таблицы — при откате это всё не будет мешать.


            1. prs123
              08.12.2018 19:02

              старый apk не выложить, у него код версии будет ниже и тогда нужно будет пользователю удалять свежую и руками ставить старую


              1. Hardcoin
                09.12.2018 10:06

                Ну разумеется версию нужно сменить. Это рокет сайнс что ли? Имеется ввиду, разработчик сменит, а не каждый пользователь.


  1. JustDont
    07.12.2018 17:54
    +3

    Отличная иллюстративная статья про восхитительный мир фронтэнда, спасибо.

    > Мы обновили 3rd-party либы и убили наше приложение для 10% юзеров
    > Но всё нормально, потому что мы быстро это пофиксили, да и вообще, шо вы хотите, граждане, нонче всё так работает.

    ^_^


    1. worldxaker
      07.12.2018 18:35

      не убили. просто в 10% случаев юзеру придется открыть приложение ещё раз


      1. JustDont
        07.12.2018 19:54
        +1

        Ну да, это конечно гораздо менее страшно, но посыл в общем остаётся тот же. Прям как будто возвращаешься в 90-е, где свойство некоторых программ «неожиданно падать» воспринималось довольно обыденно :-)


    1. dm9
      08.12.2018 15:47
      +1

      Но всё нормально, потому что мы быстро это пофиксили

      10 дней всё длилось, судя по графику. Не быстро.


  1. Xalium
    07.12.2018 21:50

    интересно, а статические анализаторы этот косяк бы нашли?


    1. ToSHiC
      08.12.2018 01:01

      Маловероятно, тут была классическая гонка, для этого теоретически существуют специальные инструменты типа fbinfer.com/docs/racerd.html, но они не гарантируют 100% поимки подобных проблем. А уж если у вас в коде есть lockless контейнеры, то дебаг будет особенно приятным :)


      1. Xalium
        08.12.2018 10:45

        ну норм. анализатор должен определять, что появилась гонка, и как-то сигнализировать по части потоконебезопасного кода.


  1. Rast1234
    08.12.2018 16:06

    Сразу подумал что race condition, когда дошли до значения -1 то и думать не надо дальше. Молодцы что нашли что и где конкретно, но ведь фикса могло и не существовать, а самим по запаре сделать фикс в чужую огромную кодбазу не получится, можно еще больше сломать. Надо было откатывать.


  1. QtRoS
    08.12.2018 17:25
    +2

    О чем статья-то? Про race condition?.. Или про то, что после обновления библиотеки надо тестить получше? Не слабовато для Хабра?


  1. Revertis
    10.12.2018 13:02

    Ммм, ясно. Хочешь разрабатывать приложения на JavaScript — готовься фиксить баги в Java.