Но ужас-то какой: через несколько часов внезапно увеличивается количество сбоев под 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, но вряд ли дело в этом.
На данный момент мы не можем воспроизвести ошибку, поэтому лучшая стратегия:
- Откатить обратно одну из двух библиотек.Выкатить её для 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)
JustDont
07.12.2018 17:54+3Отличная иллюстративная статья про восхитительный мир фронтэнда, спасибо.
> Мы обновили 3rd-party либы и убили наше приложение для 10% юзеров
> Но всё нормально, потому что мы быстро это пофиксили, да и вообще, шо вы хотите, граждане, нонче всё так работает.
^_^worldxaker
07.12.2018 18:35не убили. просто в 10% случаев юзеру придется открыть приложение ещё раз
JustDont
07.12.2018 19:54+1Ну да, это конечно гораздо менее страшно, но посыл в общем остаётся тот же. Прям как будто возвращаешься в 90-е, где свойство некоторых программ «неожиданно падать» воспринималось довольно обыденно :-)
dm9
08.12.2018 15:47+1Но всё нормально, потому что мы быстро это пофиксили
10 дней всё длилось, судя по графику. Не быстро.
Xalium
07.12.2018 21:50интересно, а статические анализаторы этот косяк бы нашли?
ToSHiC
08.12.2018 01:01Маловероятно, тут была классическая гонка, для этого теоретически существуют специальные инструменты типа fbinfer.com/docs/racerd.html, но они не гарантируют 100% поимки подобных проблем. А уж если у вас в коде есть lockless контейнеры, то дебаг будет особенно приятным :)
Xalium
08.12.2018 10:45ну норм. анализатор должен определять, что появилась гонка, и как-то сигнализировать по части потоконебезопасного кода.
Rast1234
08.12.2018 16:06Сразу подумал что race condition, когда дошли до значения -1 то и думать не надо дальше. Молодцы что нашли что и где конкретно, но ведь фикса могло и не существовать, а самим по запаре сделать фикс в чужую огромную кодбазу не получится, можно еще больше сломать. Надо было откатывать.
QtRoS
08.12.2018 17:25+2О чем статья-то? Про race condition?.. Или про то, что после обновления библиотеки надо тестить получше? Не слабовато для Хабра?
Revertis
10.12.2018 13:02Ммм, ясно. Хочешь разрабатывать приложения на JavaScript — готовься фиксить баги в Java.
alex_zzzz
Интересно, чем плоха была стратегия откатить бедных пользователей с проблемного билда на нормальный и разбираться со своими проблемами самостоятельно.
Javian
офф похоже там работают серийные программисты habr.com/post/309388
Drag13
Ну как, «откатить». Можно только выпустить новую версию либы и попросить людей обновиться самостоятельно. Всех это не спасет, но хоть что-то. А тут вообще нехорошо получилось.
mkshma
Эти люди разрабатывают под Android на реакте. Уже на этом месте стоило понять, что они занимаются чем-то неадекватным и практики у них будут соответствующие.
zagayevskiy
А что, вы знаете простой способ это сделать?
Hardcoin
Нормальный способ выложить свежую версию, которая по факту будет предыдущей? Это тривиально — способ ровно такой, каким они выложили эту бажную.
zagayevskiy
Ну-ну, ещё не забыть правильно всё откатить. Ещё бывают либы, которые не умеют. даунгредиться.
Hardcoin
Возможно я что-то не знаю про разработку под Андроид. Я полагал, что пакет приложения заменяется целиком, это не так?
Вопрос же был не как вернусь из репозитория версию до обновления (это уж точно не должно вызвать сложностей), а как обновить у пользователей? Так вот если они предыдущую версию зальют (поставив ей версию +1) — Гугл не может это залить пользователям? У них не заведётся?
zagayevskiy
Апк-то заменится. А вот пользовательские данные, в общем случае, нужно будет откатывать руками. И не всегда возможно контролировать все данные, которые записали в бд/преференсы все зависимости. Вы рассматриваете самый простой случай, а в жизни всё сложнее, и если не закладываться на даунгрейд(а я не знаю, кто в мобильной разработке на него закладывается), то откатить будет сложно, и багов на этом можно огрести гораздо больше.
Hardcoin
Зависит от библиотек, конечно, согласен. Если библиотека потребовала изменения формата внутреннего хранения (и при обновлении, например, формат сконвертировался), то это сложности. Или менеджер потребовал какую-то фичу внедрить в тот же релиз.
Про бакэнд разработку скажу, что изменения должны быть неразрушающие по возможности. Добавили колонки, таблицы — при откате это всё не будет мешать.
prs123
старый apk не выложить, у него код версии будет ниже и тогда нужно будет пользователю удалять свежую и руками ставить старую
Hardcoin
Ну разумеется версию нужно сменить. Это рокет сайнс что ли? Имеется ввиду, разработчик сменит, а не каждый пользователь.