Skip to content
This repository has been archived by the owner on Apr 24, 2022. It is now read-only.

Correction for late binding to local listener on api server #1227

Merged
merged 1 commit into from
Jun 6, 2018
Merged

Correction for late binding to local listener on api server #1227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion ethminer/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,12 @@ class MinerCLI

// Run CLI in loop
while (g_running && mgr.isRunning()) {

// Wait at the beginning of the loop to give some time
// services to start properly. Otherwise we get a "not-connected"
// message immediately
this_thread::sleep_for(chrono::seconds(m_displayInterval));

if (mgr.isConnected()) {
auto mp = f.miningProgress(m_show_hwmonitors, m_show_power);
minelog << mp << f.getSolutionStats() << ' ' << f.farmLaunchedFormatted();
Expand All @@ -734,9 +740,16 @@ class MinerCLI
else {
minelog << "not-connected";
}
this_thread::sleep_for(chrono::seconds(m_displayInterval));

}

#if API_CORE

// Stop Api server
api.stop();

#endif

mgr.stop();
stop_io_service();

Expand Down
27 changes: 24 additions & 3 deletions libapicore/ApiServer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,37 @@ void ApiServer::start()
if (m_portnumber == 0) return;

m_running.store(true, std::memory_order_relaxed);
m_acceptor.bind(tcp::endpoint(tcp::v4(), m_portnumber));

cnote << "Api server listening for connections on port " + to_string(m_acceptor.local_endpoint().port());
tcp::endpoint endpoint(tcp::v4(), m_portnumber);

// Try to bind to port number
// if exception occurs it may be due to the fact that
// requested port is already in use by another service
try
{
m_acceptor.open(endpoint.protocol());
m_acceptor.bind(endpoint);
m_acceptor.listen(64);
}
catch (const std::exception& _e)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AndreaLanfranchi: _e is unreferenced here; MSVC complains about it :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a very strange complaint as it's quite common standard to catch exception by reference. Do you mean it wants it by value ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

http://www.cplusplus.com/doc/tutorial/exceptions/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the CI https://ci.appveyor.com/project/ethereum-mining/ethminer/build/job/9573u5xgtp57xatv#L974

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just using one of MSVC's macros like UNREFERENCED_PARAMETER(_e); should do the trick.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will the directive be accepted by other compilers ? Or should I enclose it in a conditional block if _WIN32 ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This macro is MSVC specific, so we probably need to wrap it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just omit the name _e.

Copy link
Contributor

@jean-m-cyr jean-m-cyr Jun 15, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chfast approach is commonly used. ie. catch (const std::exception&)

Or, you can just add a "(void)_ec;" statement in the block. It doesn't generate any machine code and will satisfy the compiler. It's how you did it in K&R C.

{
cwarn << "Could not start API server on port : " + to_string(m_acceptor.local_endpoint().port());
cwarn << "Ensure port is not in use by another service";
return;
}

cnote << "Api server listening for connections on port " + to_string(m_acceptor.local_endpoint().port());
m_workThread = std::thread{ boost::bind(&ApiServer::begin_accept, this) };

}

void ApiServer::stop()
{
// Exit if not started
if (!m_running.load(std::memory_order_relaxed)) return;

m_acceptor.cancel();
m_acceptor.close();
m_running.store(false, std::memory_order_relaxed);

// Dispose all sessions (if any)
Expand Down Expand Up @@ -59,9 +79,10 @@ void ApiServer::handle_accept(std::shared_ptr<ApiConnection> session, boost::sys
}

});
dev::setThreadName("Api");
session->start();
m_sessions.push_back(session);
cnote << "New api session from" << session->socket().remote_endpoint();
cnote << "New api session from " << session->socket().remote_endpoint();

}
else {
Expand Down