LINUX.ORG.RU

C++ адрес подменяется

 , ,


0

1

Продолжаю изучать с++

Process* ffplay = nullptr; //указатель в 0
ffplay = new Process("xxx"); //возвращается адрес  0x00000000003aa270 и заполняется память
ffplay->Start(); //тут запускается дочерний процес и также новый thread, который читает трубу процесса

delete ffplay; //освобождаю память
ffplay = nullptr; //указываю на адрес памяти 0x0000000000000000

//далее, в триде, проверяется что объект убит (это можно определить по свойствам объекта, например там id будет -12334214, а у созданого объекта 1..2..3...) и все поля либо NULL, ну в общем понятно. 

//трид успешно завершается, говорит что объект более не живучий и выхожу из цикла чтения. Ну либо когда труба закрыта

//стоит мне после этого создать новый объект на старый указатель
ffplay = new Process("xxx"); //тут снова 0x00000000003aa270

//то потом тред не завершается, потому что считает что объект все ещё живой. Это что получается, быстренько записали на старый адрес новые данные и тред даже не знает что ему подменили объект?

//если следом создать ещё один
Process *ffplay2 = new Process("xxx"); //тут адрес уже другой 0x000000000022d6b0


Как мне избежать такой подмены объекта в треде?

★★★★

ffplay = nullptr; //указываю на адрес памяти 0x0000000000000000
...
ffplay = new Process("xxx"); //тут снова 0x00000000003aa270

Между этими двумя действиями в этом же треде есть код который проверяет ffplay? Если нет - то компилятор может выкинуть =null, если есть, то компилятор может выкинуть проверку (потому что там null), а потом см. пункт А.

PPP328 ★★★ ()
Ответ на: комментарий от gobot

Потому что ты написал delete ffplay. То есть сообщил, что старый адрес теперь свободен и по нему можно записать новый объект.

в триде, проверяется что объект убит (это можно определить по свойствам объекта)

Не надо так делать. Это UB вообще-то. Надо сначала завершать поток, а уже потом освобождать память.

monk ★★★★★ ()

кажись понял!

Как мне избежать такой подмены объекта в треде?

тебе надо другую переменную использовать, либо перед созданием нового объекта сделать thread.Join
а ещё везде перед
ffplay = new Process(«xxx»);
захватить мьютекс, а после отхватить обратно

и переменную Process* ffplay = nullptr;
сделать atomic или volatile

Bad_ptr ★★★★ ()
Последнее исправление: Bad_ptr (всего исправлений: 1)

зачем тут вообще используются указатели?

Process ffplay("xxx");
ffplay.Start();

далее, в триде, проверяется что объект убит (это можно определить по свойствам объекта, например там id будет -12334214, а у созданого объекта 1..2..3…)

так нельзя делать

Lrrr ()
Ответ на: кажись понял! от Bad_ptr

Переменная кстати не влияет. Я пробовал просто new Process(«zzzzzz») и все равно в старый адрес записывается

Правильно я предполагаю, что данные перезатерлись и теперь тред видит новый объект? Может сделать обнаружение «подмены»? Например в начале записать id объекта, а потом в цикле его проверять

//start thread
int id = self->id;
for (;;) {
  if(self->isClosed() || self->id != id) return error("Объект подменен или убит");
}


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

gobot ★★★★ ()
Последнее исправление: gobot (всего исправлений: 1)
Ответ на: комментарий от gobot

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

Максимально проще перед delete давать команду потоку на завершение и ждать его завершения. Если ты освободил память, но поток в ней продолжает работать, то может быть что угодно: от падения программы (если процесс честно отдал память операционной системе) до изменения произвольных данных в других местах программы (новый объект на том же месте совсем не обязан быть таким же процессом).

monk ★★★★★ ()
Ответ на: комментарий от gobot

Я не хочу принудительно убивать тред.

Можешь не принудительно. Ставишь объекту флажок завершения, а потом ждёшь пока поток просигнализирует, что закончился.

monk ★★★★★ ()

далее, в триде, проверяется что объект убит (это можно определить по свойствам объекта, например там id будет -12334214, а у созданого объекта 1..2..3…)

Скорее всего, это undefined behaviour. Точно сказать можно будет только при виде кода того дочернего потока, но по симптомам похоже на use-after-free. После UB никаких гарантий про выполнение программы дать уже нельзя. Вам придётся переписать дочерний поток так, чтобы он не вызывал UB.

anonymous ()
Ответ на: комментарий от gobot

Может сделать обнаружение «подмены»?

Надо сделать так, чтобы вообще не обращаться к потенциально «мёртвым» или «подменённым» объектам.

Как вариант, можно в объекте ставить где-то флаг, что им нельзя больше пользоваться. Удалять объект тогда должен последний потенциальный пользователь. Если речь идёт о доступе из нескольких тредов, надо будет задуматься о возможных последствиях одновременного доступа; и да, там будут, скорее всего, atomic-переменные или мьютексы.

В целом не совсем понимаю, что тут происходит, поэтому точнее подсказать не могу. Есть ощущение, что твои муки сейчас целиком из серии «Чем забить гвоздь: ботинком или бутылкой?».

proud_anon ★★★★★ ()
Ответ на: комментарий от anonymous

На rust и нормальный код не компилируется:

use std::thread;

fn main() {
    let v = vec![1, 2, 3];

    let handle = thread::spawn(|| {
        println!("Here's a vector: {:?}", v);
    });

    handle.join().unwrap();
    drop(v);
}

Он не понимает, что к моменту drop поток уже гарантированно завершился.

monk ★★★★★ ()

тебе лучше разделить логику, ввод/вывод и потоки. чтобы там где ты удаляешь процесс ты бы работал только с потоком, а поток у себя внутри уже работал бы с пайпами. сам факт того, что ты можешь что-то подобное сделать говорит о том, что у тебя не очень прямая архитектура, а твоя ошибка видимо в том, что ты спрятал поток в объекте. но для начала пойдёт наверное, так действительно будет проще и быстрее.

и поставь себе валгринд уже. ты на винде небось, ну поставь виртуалку, в виртуалку линукс, а в линукс валгринд.

anonymous ()
Ответ на: комментарий от gobot

Переменная кстати не влияет. Я пробовал просто new Process(«zzzzzz») и все равно в старый адрес записывается

размер не-шаблонного объекта всегда постоянный и у экземпляров Process он всегда будет одинаковый. скорее всего ты используешь внутри Process какой-нибудь std::string, его размер тоже всегда одинаковый, а память для самой строки он алоцирует в куче. т.е. сам объект может быть и на стеке, а строка будет жить в куче.

если ты используешь const char*, то размер этого указателя тоже будет одинаковым в разных объектах, а сама строка будет жить там куда указывает указатель.

anonymous ()
Ответ на: комментарий от monk

Rust ничего не может понимать. Оно не так работает. Там никакого статанализа. Всё это написано на unsafe и ничего не знает о контексте и окружени.

anonymous ()
Ответ на: комментарий от gobot
  1. Либо оптимизирует компилятор, видя, что вы высвобождаете память под объект одного и того же размера и вида и мог просто передать эту память. По идее ели уберёте:
ffplay = new Process("xxx");

То он может передать этот адрес ffplay2. В целом это не гарантированное поведение.

  1. Либо так отрабатывает штатный delete на вашей системе - вы ему по сути сказали что освободили, он дабы уменьшить фрагментацию сохранил высвобожденный участок, который затем подошёл под новый блок пямяти.
AKonia ()
Последнее исправление: AKonia (всего исправлений: 1)
Ответ на: комментарий от gobot

Ну ты старый адрес освободил, аллокатор пометил его как свободный и, когда ты сделал новый объект, аллокатор его там же и создал. Память-то свободна, херли бы и не создать?

hateyoufeel ★★★★★ ()
Ответ на: комментарий от anonymous

В точке где вызывается drop(v); переменой v уже не существует, ты же передал владение v в лямбду.

Не передал, а на время отдал. Вот так работает:

fn main() {
    let v = vec![1, 2, 3];

    let handle = || {
        println!("Here's a vector: {:?}", v);
    };

    handle();
    drop(v);
}
monk ★★★★★ ()

Как уже многие сказали, никак ты этого не избежишь. Потому что делаешь неправильно. Надо не завершать тред, когда объект убит; надо убивать объект только после того, как завершится тред. delete предназначен только для того, чтобы освободить ненужную память; для сигнализации кому-то, что объект более не валиден, delete не годится.

Miguel ★★★★★ ()

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

Но если поток находится внутри Process, то мне не понятно как он получает указатель на класс Process. Или ты this сравниваешь с нулём? Это тоже плохо.

Тебе нужно добавить флаг, например std::atomic_bool или обычная переменная с мьютексом, если он уже у тебя есть. Флаг постоянно проверяй внутри цикла потока и выходи, если он поднят. Далее в деструкторе Process поднимаешь флаг и делаешь join() на потоке. Теперь при удалении Process, поток гарантированно будет остановлен.

ox55ff ★★★ ()
например там id будет -12334214

Я правильно понимаю, что ты смотришь свойства объекта, для которого уже сделал delete? Если да, то это фееричный выстрел в ногу

Aswed ★★★★★ ()